Merge pull request #6341 from libgit2/ethomson/ownership2

Fix erroneously lax configuration ownership checks

5 files changed, 174 insertions, 109 deletions

diff --git a/src/libgit2/config.c b/src/libgit2/config.c
index bce21d712..5c366e221 100644
--- a/src/libgit2/config.c
+++ b/src/libgit2/config.c
@@ -1170,10 +1170,13 @@ int git_config_find_programdata(git_buf *path)
 
 int git_config__find_programdata(git_str *path)
 {
+	git_fs_path_owner_t owner_level =
+		GIT_FS_PATH_OWNER_CURRENT_USER |
+		GIT_FS_PATH_OWNER_ADMINISTRATOR;
 	bool is_safe;
 
 	if (git_sysdir_find_programdata_file(path, GIT_CONFIG_FILENAME_PROGRAMDATA) < 0 ||
-	    git_fs_path_owner_is_system_or_current_user(&is_safe, path->ptr) < 0)
+	    git_fs_path_owner_is(&is_safe, path->ptr, owner_level) < 0)
 		return -1;
 
 	if (!is_safe) {
diff --git a/src/libgit2/repository.c b/src/libgit2/repository.c
index d2484318f..09e81cded 100644
--- a/src/libgit2/repository.c
+++ b/src/libgit2/repository.c
@@ -509,10 +509,13 @@ static int validate_ownership(const char *repo_path)
 {
 	git_config *config = NULL;
 	validate_ownership_data data = { repo_path, GIT_STR_INIT, false };
+	git_fs_path_owner_t owner_level =
+		GIT_FS_PATH_OWNER_CURRENT_USER |
+		GIT_FS_PATH_USER_IS_ADMINISTRATOR;
 	bool is_safe;
 	int error;
 
-	if ((error = git_fs_path_owner_is_system_or_current_user(&is_safe, repo_path)) < 0) {
+	if ((error = git_fs_path_owner_is(&is_safe, repo_path, owner_level)) < 0) {
 		if (error == GIT_ENOTFOUND)
 			error = 0;
 
diff --git a/src/util/fs_path.c b/src/util/fs_path.c
index 9bd773d27..1eb41ef84 100644
--- a/src/util/fs_path.c
+++ b/src/util/fs_path.c
@@ -1785,9 +1785,9 @@ done:
 	return supported;
 }
 
-static git_fs_path__mock_owner_t mock_owner = GIT_FS_PATH_MOCK_OWNER_NONE;
+static git_fs_path_owner_t mock_owner = GIT_FS_PATH_OWNER_NONE;
 
-void git_fs_path__set_owner(git_fs_path__mock_owner_t owner)
+void git_fs_path__set_owner(git_fs_path_owner_t owner)
 {
 	mock_owner = owner;
 }
@@ -1879,74 +1879,52 @@ static int file_owner_sid(PSID *out, const char *path)
 	return error;
 }
 
-int git_fs_path_owner_is_current_user(bool *out, const char *path)
+int git_fs_path_owner_is(
+	bool *out,
+	const char *path,
+	git_fs_path_owner_t owner_type)
 {
 	PSID owner_sid = NULL, user_sid = NULL;
-	int error = -1;
+	BOOL is_admin, admin_owned;
+	int error;
 
 	if (mock_owner) {
-		*out = (mock_owner == GIT_FS_PATH_MOCK_OWNER_CURRENT_USER);
+		*out = ((mock_owner & owner_type) != 0);
 		return 0;
 	}
 
-	if ((error = file_owner_sid(&owner_sid, path)) < 0 ||
-	    (error = current_user_sid(&user_sid)) < 0)
+	if ((error = file_owner_sid(&owner_sid, path)) < 0)
 		goto done;
 
-	*out = EqualSid(owner_sid, user_sid);
-	error = 0;
+	if ((owner_type & GIT_FS_PATH_OWNER_CURRENT_USER) != 0) {
+		if ((error = current_user_sid(&user_sid)) < 0)
+			goto done;
 
-done:
-	git__free(owner_sid);
-	git__free(user_sid);
-	return error;
-}
-
-int git_fs_path_owner_is_system(bool *out, const char *path)
-{
-	PSID owner_sid;
-
-	if (mock_owner) {
-		*out = (mock_owner == GIT_FS_PATH_MOCK_OWNER_SYSTEM);
-		return 0;
-	}
-
-	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_fs_path_owner_is_system_or_current_user(bool *out, const char *path)
-{
-	PSID owner_sid = NULL, user_sid = NULL;
-	int error = -1;
-
-	if (mock_owner) {
-		*out = (mock_owner == GIT_FS_PATH_MOCK_OWNER_SYSTEM ||
-		        mock_owner == GIT_FS_PATH_MOCK_OWNER_CURRENT_USER);
-		return 0;
+		if (EqualSid(owner_sid, user_sid)) {
+			*out = true;
+			goto done;
+		}
 	}
 
-	if (file_owner_sid(&owner_sid, path) < 0)
-		goto done;
+	admin_owned =
+		IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) ||
+		IsWellKnownSid(owner_sid, WinLocalSystemSid);
 
-	if (IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) ||
-	    IsWellKnownSid(owner_sid, WinLocalSystemSid)) {
-		*out = 1;
-		error = 0;
+	if (admin_owned &&
+	    (owner_type & GIT_FS_PATH_OWNER_ADMINISTRATOR) != 0) {
+		*out = true;
 		goto done;
 	}
 
-	if (current_user_sid(&user_sid) < 0)
+	if (admin_owned &&
+	    (owner_type & GIT_FS_PATH_USER_IS_ADMINISTRATOR) != 0 &&
+	    CheckTokenMembership(NULL, owner_sid, &is_admin) &&
+	    is_admin) {
+		*out = true;
 		goto done;
+	}
 
-	*out = EqualSid(owner_sid, user_sid);
-	error = 0;
+	*out = false;
 
 done:
 	git__free(owner_sid);
@@ -1956,10 +1934,25 @@ done:
 
 #else
 
-static int fs_path_owner_is(bool *out, const char *path, uid_t *uids, size_t uids_len)
+int git_fs_path_owner_is(
+	bool *out,
+	const char *path,
+	git_fs_path_owner_t owner_type)
 {
+	uid_t uids[2] = { 0 };
+	size_t uid_count = 0, i;
 	struct stat st;
-	size_t i;
+
+	if (mock_owner) {
+		*out = ((mock_owner & owner_type) != 0);
+		return 0;
+	}
+
+	if (owner_type & GIT_FS_PATH_OWNER_CURRENT_USER)
+		uids[uid_count++] = geteuid();
+
+	if (owner_type & GIT_FS_PATH_OWNER_ADMINISTRATOR)
+		uids[uid_count++] = 0;
 
 	*out = false;
 
@@ -1971,7 +1964,7 @@ static int fs_path_owner_is(bool *out, const char *path, uid_t *uids, size_t uid
 		return -1;
 	}
 
-	for (i = 0; i < uids_len; i++) {
+	for (i = 0; i < uid_count; i++) {
 		if (uids[i] == st.st_uid) {
 			*out = true;
 			break;
@@ -1981,45 +1974,18 @@ static int fs_path_owner_is(bool *out, const char *path, uid_t *uids, size_t uid
 	return 0;
 }
 
+#endif
+
 int git_fs_path_owner_is_current_user(bool *out, const char *path)
 {
-	uid_t userid = geteuid();
-
-	if (mock_owner) {
-		*out = (mock_owner == GIT_FS_PATH_MOCK_OWNER_CURRENT_USER);
-		return 0;
-	}
-
-	return fs_path_owner_is(out, path, &userid, 1);
+	return git_fs_path_owner_is(out, path, GIT_FS_PATH_OWNER_CURRENT_USER);
 }
 
 int git_fs_path_owner_is_system(bool *out, const char *path)
 {
-	uid_t userid = 0;
-
-	if (mock_owner) {
-		*out = (mock_owner == GIT_FS_PATH_MOCK_OWNER_SYSTEM);
-		return 0;
-	}
-
-	return fs_path_owner_is(out, path, &userid, 1);
-}
-
-int git_fs_path_owner_is_system_or_current_user(bool *out, const char *path)
-{
-	uid_t userids[2] = { geteuid(), 0 };
-
-	if (mock_owner) {
-		*out = (mock_owner == GIT_FS_PATH_MOCK_OWNER_SYSTEM ||
-		        mock_owner == GIT_FS_PATH_MOCK_OWNER_CURRENT_USER);
-		return 0;
-	}
-
-	return fs_path_owner_is(out, path, userids, 2);
+	return git_fs_path_owner_is(out, path, GIT_FS_PATH_OWNER_ADMINISTRATOR);
 }
 
-#endif
-
 int git_fs_path_find_executable(git_str *fullpath, const char *executable)
 {
 #ifdef GIT_WIN32
diff --git a/src/util/fs_path.h b/src/util/fs_path.h
index cf4664178..fd045a313 100644
--- a/src/util/fs_path.h
+++ b/src/util/fs_path.h
@@ -732,18 +732,37 @@ int git_fs_path_normalize_slashes(git_str *out, const char *path);
 bool git_fs_path_supports_symlinks(const char *dir);
 
 typedef enum {
-	GIT_FS_PATH_MOCK_OWNER_NONE = 0, /* do filesystem lookups as normal */
-	GIT_FS_PATH_MOCK_OWNER_SYSTEM = 1,
-	GIT_FS_PATH_MOCK_OWNER_CURRENT_USER = 2,
-	GIT_FS_PATH_MOCK_OWNER_OTHER = 3
-} git_fs_path__mock_owner_t;
+	GIT_FS_PATH_OWNER_NONE = 0,
+
+	/** The file must be owned by the current user. */
+	GIT_FS_PATH_OWNER_CURRENT_USER = (1 << 0),
+
+	/** The file must be owned by the system account. */
+	GIT_FS_PATH_OWNER_ADMINISTRATOR = (1 << 1),
+
+	/**
+	 * The file may be owned by a system account if the current
+	 * user is in an administrator group. Windows only; this is
+	 * a noop on non-Windows systems.
+	 */
+	GIT_FS_PATH_USER_IS_ADMINISTRATOR = (1 << 2),
+
+	/** The file may be owned by another user. */
+	GIT_FS_PATH_OWNER_OTHER = (1 << 3)
+} git_fs_path_owner_t;
 
 /**
  * Sets the mock ownership for files; subsequent calls to
- * `git_fs_path_owner_is_*` functions will return this data until cleared
- * with `GIT_FS_PATH_MOCK_OWNER_NONE`.
+ * `git_fs_path_owner_is_*` functions will return this data until
+ * cleared with `GIT_FS_PATH_OWNER_NONE`.
  */
-void git_fs_path__set_owner(git_fs_path__mock_owner_t owner);
+void git_fs_path__set_owner(git_fs_path_owner_t owner);
+
+/** Verify that the file in question is owned by the given owner. */
+int git_fs_path_owner_is(
+	bool *out,
+	const char *path,
+	git_fs_path_owner_t owner_type);
 
 /**
  * Verify that the file in question is owned by an administrator or system
@@ -758,12 +777,6 @@ int git_fs_path_owner_is_system(bool *out, const char *path);
 int git_fs_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_fs_path_owner_is_system_or_current_user(bool *out, const char *path);
-
-/**
  * Search the current PATH for the given executable, returning the full
  * path if it is found.
  */
diff --git a/tests/libgit2/repo/open.c b/tests/libgit2/repo/open.c
index 5c66eca4b..634ba59d2 100644
--- a/tests/libgit2/repo/open.c
+++ b/tests/libgit2/repo/open.c
@@ -16,12 +16,13 @@ void test_repo_open__cleanup(void)
 {
 	cl_git_sandbox_cleanup();
 	cl_fixture_cleanup("empty_standard_repo");
+	cl_fixture_cleanup("testrepo.git");
 	cl_fixture_cleanup("__global_config");
 
 	if (git_fs_path_isdir("alternate"))
 		git_futils_rmdir_r("alternate", NULL, GIT_RMDIR_REMOVE_FILES);
 
-	git_fs_path__set_owner(GIT_FS_PATH_MOCK_OWNER_NONE);
+	git_fs_path__set_owner(GIT_FS_PATH_OWNER_NONE);
 
 	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, config_path.ptr));
 	git_buf_dispose(&config_path);
@@ -480,20 +481,55 @@ void test_repo_open__validates_dir_ownership(void)
 	cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git"));
 
 	/* When the current user owns the repo config, that's acceptable */
-	git_fs_path__set_owner(GIT_FS_PATH_MOCK_OWNER_CURRENT_USER);
+	git_fs_path__set_owner(GIT_FS_PATH_OWNER_CURRENT_USER);
 	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
 	git_repository_free(repo);
 
-	/* When the system user owns the repo config, also acceptable */
-	git_fs_path__set_owner(GIT_FS_PATH_MOCK_OWNER_SYSTEM);
+	/* When the system user owns the repo config, fail */
+	git_fs_path__set_owner(GIT_FS_PATH_OWNER_ADMINISTRATOR);
+	cl_git_fail(git_repository_open(&repo, "empty_standard_repo"));
+
+#ifdef GIT_WIN32
+	/* When the user is an administrator, succeed on Windows. */
+	git_fs_path__set_owner(GIT_FS_PATH_USER_IS_ADMINISTRATOR);
 	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
 	git_repository_free(repo);
+#endif
 
 	/* When an unknown user owns the repo config, fail */
-	git_fs_path__set_owner(GIT_FS_PATH_MOCK_OWNER_OTHER);
+	git_fs_path__set_owner(GIT_FS_PATH_OWNER_OTHER);
 	cl_git_fail(git_repository_open(&repo, "empty_standard_repo"));
 }
 
+void test_repo_open__validates_bare_repo_ownership(void)
+{
+	git_repository *repo;
+
+	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_OWNER_VALIDATION, 1));
+
+	cl_fixture_sandbox("testrepo.git");
+
+	/* When the current user owns the repo config, that's acceptable */
+	git_fs_path__set_owner(GIT_FS_PATH_OWNER_CURRENT_USER);
+	cl_git_pass(git_repository_open(&repo, "testrepo.git"));
+	git_repository_free(repo);
+
+	/* When the system user owns the repo config, fail */
+	git_fs_path__set_owner(GIT_FS_PATH_OWNER_ADMINISTRATOR);
+	cl_git_fail(git_repository_open(&repo, "testrepo.git"));
+
+#ifdef GIT_WIN32
+	/* When the user is an administrator, succeed on Windows. */
+	git_fs_path__set_owner(GIT_FS_PATH_USER_IS_ADMINISTRATOR);
+	cl_git_pass(git_repository_open(&repo, "testrepo.git"));
+	git_repository_free(repo);
+#endif
+
+	/* When an unknown user owns the repo config, fail */
+	git_fs_path__set_owner(GIT_FS_PATH_OWNER_OTHER);
+	cl_git_fail(git_repository_open(&repo, "testrepo.git"));
+}
+
 void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void)
 {
 	git_repository *repo;
@@ -506,7 +542,7 @@ void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void)
 	cl_fixture_sandbox("empty_standard_repo");
 	cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git"));
 
-	git_fs_path__set_owner(GIT_FS_PATH_MOCK_OWNER_OTHER);
+	git_fs_path__set_owner(GIT_FS_PATH_OWNER_OTHER);
 	cl_git_fail(git_repository_open(&repo, "empty_standard_repo"));
 
 	/* Add safe.directory options to the global configuration */
@@ -539,6 +575,50 @@ void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void)
 	git_str_dispose(&config_data);
 }
 
+void test_repo_open__can_allowlist_bare_gitdir(void)
+{
+	git_repository *repo;
+	git_str config_path = GIT_STR_INIT,
+	        config_filename = GIT_STR_INIT,
+	        config_data = GIT_STR_INIT;
+
+	cl_git_pass(git_libgit2_opts(GIT_OPT_SET_OWNER_VALIDATION, 1));
+
+	cl_fixture_sandbox("testrepo.git");
+
+	git_fs_path__set_owner(GIT_FS_PATH_OWNER_OTHER);
+	cl_git_fail(git_repository_open(&repo, "testrepo.git"));
+
+	/* Add safe.directory options to the global configuration */
+	git_str_joinpath(&config_path, clar_sandbox_path(), "__global_config");
+	cl_must_pass(p_mkdir(config_path.ptr, 0777));
+	git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, config_path.ptr);
+
+	git_str_joinpath(&config_filename, config_path.ptr, ".gitconfig");
+
+	git_str_printf(&config_data,
+		"[foo]\n" \
+		"\tbar = Foobar\n" \
+		"\tbaz = Baz!\n" \
+		"[safe]\n" \
+		"\tdirectory = /non/existent/path\n" \
+		"\tdirectory = /\n" \
+		"\tdirectory = c:\\\\temp\n" \
+		"\tdirectory = %s/%s\n" \
+		"\tdirectory = /tmp\n" \
+		"[bar]\n" \
+		"\tfoo = barfoo\n",
+		clar_sandbox_path(), "testrepo.git");
+	cl_git_rewritefile(config_filename.ptr, config_data.ptr);
+
+	cl_git_pass(git_repository_open(&repo, "testrepo.git"));
+	git_repository_free(repo);
+
+	git_str_dispose(&config_path);
+	git_str_dispose(&config_filename);
+	git_str_dispose(&config_data);
+}
+
 void test_repo_open__can_reset_safe_directory_list(void)
 {
 	git_repository *repo;
@@ -551,7 +631,7 @@ void test_repo_open__can_reset_safe_directory_list(void)
 	cl_fixture_sandbox("empty_standard_repo");
 	cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git"));
 
-	git_fs_path__set_owner(GIT_FS_PATH_MOCK_OWNER_OTHER);
+	git_fs_path__set_owner(GIT_FS_PATH_OWNER_OTHER);
 	cl_git_fail(git_repository_open(&repo, "empty_standard_repo"));
 
 	/* Add safe.directory options to the global configuration */