path: refactor ownership checks into current user and system

Provide individual file ownership checks for both the current user and the system user, as well as a combined current user and system user check.
nodegit · julianmesa-gitkraken · Mar 29, 2021 · Aug 12, 2020 · Mar 29, 2021 · Aug 12, 2020
commit 973d959abaa5faaddec6a4c27889ff5b980d2550
diff --git a/src/config.c b/src/config.c
@@ -1118,16 +1118,20 @@ int git_config_find_system(git_buf *path)
 int git_config_find_programdata(git_buf *path)
 {
 	int ret;
+	bool is_safe;
 
-	if ((ret = git_buf_sanitize(path)) < 0)
+	if ((ret = git_buf_sanitize(path)) < 0 ||
+	    (ret = git_sysdir_find_programdata_file(path,
+	       GIT_CONFIG_FILENAME_PROGRAMDATA)) < 0 ||
+	    (ret = git_path_owner_is_system_or_current_user(&is_safe, path->ptr)) < 0)
 		return ret;
 
-	ret = git_sysdir_find_programdata_file(path,
-					       GIT_CONFIG_FILENAME_PROGRAMDATA);
-	if (ret != GIT_OK)
-		return ret;
+	if (!is_safe) {
+		git_error_set(GIT_ERROR_CONFIG, "programdata path has invalid ownership");
+		return -1;
+	}
 
-	return git_path_validate_system_file_ownership(path->ptr);
+	return 0;
 }
 
 int git_config__global_location(git_buf *buf)

diff --git a/src/path.c b/src/path.c
@@ -2024,78 +2024,195 @@ bool git_path_supports_symlinks(const char *dir)
 	return supported;
 }
 
-int git_path_validate_system_file_ownership(const char *path)
+#ifdef GIT_WIN32
+static PSID *sid_dup(PSID sid)
 {
-#ifndef GIT_WIN32
-	GIT_UNUSED(path);
-	return GIT_OK;
-#else
-	git_win32_path buf;
-	PSID owner_sid;
-	PSECURITY_DESCRIPTOR descriptor = NULL;
-	HANDLE token;
-	TOKEN_USER *info = NULL;
-	DWORD err, len;
-	int ret;
+	DWORD len;
+	PSID dup;
 
-	if (git_win32_path_from_utf8(buf, path) < 0)
-		return -1;
+	len = GetLengthSid(sid);
 
-	err = GetNamedSecurityInfoW(buf, SE_FILE_OBJECT,
-				    OWNER_SECURITY_INFORMATION |
-					    DACL_SECURITY_INFORMATION,
-				    &owner_sid, NULL, NULL, NULL, &descriptor);
+	if ((dup = git__malloc(len)) == NULL)
+		return NULL;
 
-	if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
-		ret = GIT_ENOTFOUND;
-		goto cleanup;
+	if (!CopySid(len, dup, sid)) {
+		git_error_set(GIT_ERROR_OS, "could not duplicate sid");
+		git__free(dup);
+		return NULL;
 	}
 
-	if (err != ERROR_SUCCESS) {
-		git_error_set(GIT_ERROR_OS, "failed to get security information");
-		ret = GIT_ERROR;
-		goto cleanup;
-	}
+	return dup;
+}
+
+static int current_user_sid(PSID *out)
+{
+	TOKEN_USER *info = NULL;
+	HANDLE token = NULL;
+	DWORD len = 0;
+	int error = -1;
 
-	if (!IsValidSid(owner_sid)) {
-		git_error_set(GIT_ERROR_INVALID, "programdata configuration file owner is unknown");
-		ret = GIT_ERROR;
-		goto cleanup;
+	if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token)) {
+		git_error_set(GIT_ERROR_OS, "could not lookup process information");
+		goto done;
 	}
 
-	if (IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) ||
-	    IsWellKnownSid(owner_sid, WinLocalSystemSid)) {
-		ret = GIT_OK;
-		goto cleanup;
-	}
-
-	/* Obtain current user's SID */
-	if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token) &&
-	    !GetTokenInformation(token, TokenUser, NULL, 0, &len)) {
-		info = git__malloc(len);
-		GIT_ERROR_CHECK_ALLOC(info);
-		if (!GetTokenInformation(token, TokenUser, info, len, &len)) {
-			git__free(info);
-			info = NULL;
-		}
+	if (GetTokenInformation(token, TokenUser, NULL, 0, &len) ||
+		GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
+		git_error_set(GIT_ERROR_OS, "could not lookup token metadata");
+		goto done;
 	}
 
-	/*
-	 * If the file is owned by the same account that is running the current
-	 * process, it's okay to read from that file.
-	 */
-	if (info && EqualSid(owner_sid, info->User.Sid))
-		ret = GIT_OK;
-	else {
-		git_error_set(GIT_ERROR_INVALID, "programdata configuration file owner is not valid");
-		ret = GIT_ERROR;
+	info = git__malloc(len);
+	GIT_ERROR_CHECK_ALLOC(info);
+
+	if (!GetTokenInformation(token, TokenUser, info, len, &len)) {
+		git_error_set(GIT_ERROR_OS, "could not lookup current user");
+		goto done;
 	}
+
+	if ((*out = sid_dup(info->User.Sid)))
+		error = 0;
+
+done:
+	if (token)
+		CloseHandle(token);
+
 	git__free(info);
+	return error;
+}
+
+static int file_owner_sid(PSID *out, const char *path)
+{
+	git_win32_path path_w32;
+	PSECURITY_DESCRIPTOR descriptor = NULL;
+	PSID owner_sid;
+	DWORD ret;
+	int error = -1;
+
+	if (git_win32_path_from_utf8(path_w32, path) < 0)
+		return -1;
+
+	ret = GetNamedSecurityInfoW(path_w32, SE_FILE_OBJECT,
+		OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION,
+		&owner_sid, NULL, NULL, NULL, &descriptor);
+
+	if (ret == ERROR_FILE_NOT_FOUND || ret == ERROR_PATH_NOT_FOUND)
+		error = GIT_ENOTFOUND;
+	else if (ret != ERROR_SUCCESS)
+		git_error_set(GIT_ERROR_OS, "failed to get security information");
+	else if (!IsValidSid(owner_sid))
+		git_error_set(GIT_ERROR_OS, "file owner is not valid");
+	else if ((*out = sid_dup(owner_sid)))
+		error = 0;
 
-cleanup:
 	if (descriptor)
 		LocalFree(descriptor);
 
-	return ret;
-#endif
+	return error;
 }
+
+int git_path_owner_is_current_user(bool *out, const char *path)
+{
+	PSID owner_sid = NULL, user_sid = NULL;
+	int error = -1;
+
+	if ((error = file_owner_sid(&owner_sid, path)) < 0 ||
+	    (error = current_user_sid(&user_sid)) < 0)
+		goto done;
+
+	*out = EqualSid(owner_sid, user_sid);
+	error = 0;
+
+done:
+	git__free(owner_sid);
+	git__free(user_sid);
+	return error;
+}
+
+int git_path_owner_is_system(bool *out, const char *path)
+{
+	PSID owner_sid;
+
+	if (file_owner_sid(&owner_sid, path) < 0)
+		return -1;
+
+	*out = IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) ||
+	       IsWellKnownSid(owner_sid, WinLocalSystemSid);
+
+	git__free(owner_sid);
+	return 0;
+}
+
+int git_path_owner_is_system_or_current_user(bool *out, const char *path)
+{
+	PSID owner_sid = NULL, user_sid = NULL;
+	int error = -1;
+
+	if (file_owner_sid(&owner_sid, path) < 0)
+		goto done;
+
+	if (IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) ||
+	    IsWellKnownSid(owner_sid, WinLocalSystemSid)) {
+		*out = 1;
+		error = 0;
+		goto done;
+	}
+
+	if (current_user_sid(&user_sid) < 0)
+		goto done;
+
+	*out = EqualSid(owner_sid, user_sid);
+	error = 0;
+
+done:
+	git__free(owner_sid);
+	git__free(user_sid);
+	return error;
+}
+
+#else
+
+static int path_owner_is(bool *out, const char *path, uid_t *uids, size_t uids_len)
+{
+	struct stat st;
+	size_t i;
+
+	*out = false;
+
+	if (p_lstat(path, &st) != 0) {
+		if (errno == ENOENT)
+			return GIT_ENOTFOUND;
+
+		git_error_set(GIT_ERROR_OS, "could not stat '%s'", path);
+		return -1;
+	}
+
+	for (i = 0; i < uids_len; i++) {
+		if (uids[i] == st.st_uid) {
+			*out = true;
+			break;
+		}
+	}
+
+	return 0;
+}
+
+int git_path_owner_is_current_user(bool *out, const char *path)
+{
+	uid_t userid = geteuid();
+	return path_owner_is(out, path, &userid, 1);
+}
+
+int git_path_owner_is_system(bool *out, const char *path)
+{
+	uid_t userid = 0;
+	return path_owner_is(out, path, &userid, 1);
+}
+
+int git_path_owner_is_system_or_current_user(bool *out, const char *path)
+{
+	uid_t userids[2] = { geteuid(), 0 };
+	return path_owner_is(out, path, userids, 2);
+}
+
+#endif
diff --git a/src/path.h b/src/path.h
@@ -723,15 +723,21 @@ int git_path_normalize_slashes(git_buf *out, const char *path);
 bool git_path_supports_symlinks(const char *dir);
 
 /**
- * Validate a system file's ownership
- *
  * Verify that the file in question is owned by an administrator or system
- * account, or at least by the current user.
- *
- * This function returns 0 if successful. If the file is not owned by any of
- * these, or any other if there have been problems determining the file
- * ownership, it returns -1.
+ * account.
+ */
+int git_path_owner_is_system(bool *out, const char *path);
+
+/**
+ * Verify that the file in question is owned by the current user;
+ */
+
+int git_path_owner_is_current_user(bool *out, const char *path);
+
+/**
+ * Verify that the file in question is owned by an administrator or system
+ * account _or_ the current user;
  */
-int git_path_validate_system_file_ownership(const char *path);
+int git_path_owner_is_system_or_current_user(bool *out, const char *path);
 
 #endif
diff --git a/tests/core/path.c b/tests/core/path.c
@@ -659,3 +659,28 @@ void test_core_path__git_path_is_file(void)
 	cl_git_pass(git_path_is_gitfile("blob", 4, GIT_PATH_GITFILE_GITATTRIBUTES, GIT_PATH_FS_HFS));
 	cl_git_fail(git_path_is_gitfile("blob", 4, 3, GIT_PATH_FS_HFS));
 }
+
+void test_core_path__validate_current_user_ownership(void)
+{
+	bool is_cur;
+
+	cl_must_pass(p_mkdir("testdir", 0777));
+	cl_git_pass(git_path_owner_is_current_user(&is_cur, "testdir"));
+	cl_assert_equal_i(is_cur, 1);
+
+	cl_git_rewritefile("testfile", "This is a test file.");
+	cl_git_pass(git_path_owner_is_current_user(&is_cur, "testfile"));
+	cl_assert_equal_i(is_cur, 1);
+
+#ifdef GIT_WIN32
+	cl_git_pass(git_path_owner_is_current_user(&is_cur, "C:\\"));
+	cl_assert_equal_i(is_cur, 0);
+
+	cl_git_fail(git_path_owner_is_current_user(&is_cur, "c:\\path\\does\\not\\exist"));
+#else
+	cl_git_pass(git_path_owner_is_current_user(&is_cur, "/"));
+	cl_assert_equal_i(is_cur, 0);
+
+	cl_git_fail(git_path_owner_is_current_user(&is_cur, "/path/does/not/exist"));
+#endif
+}