diff options
author | Edward Thomson <ethomson@edwardthomson.com> | 2022-07-12 14:12:15 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-07-12 14:12:15 -0400 |
commit | d1001fd05bd20b92b7568d93b0471feb3547035b (patch) | |
tree | 5e5c51830bf04567d9fc527165000a3ed12200c6 | |
parent | 92ffdd2cd243a49fafb317ea3a819dbe8a6dd3c9 (diff) | |
parent | 56aaaf532d9364cea146037023b64c2ee5e0bcb6 (diff) | |
download | libgit2-d1001fd05bd20b92b7568d93b0471feb3547035b.tar.gz |
Merge pull request #6341 from libgit2/ethomson/ownership2
Fix erroneously lax configuration ownership checks
-rw-r--r-- | src/libgit2/config.c | 5 | ||||
-rw-r--r-- | src/libgit2/repository.c | 5 | ||||
-rw-r--r-- | src/util/fs_path.c | 138 | ||||
-rw-r--r-- | src/util/fs_path.h | 41 | ||||
-rw-r--r-- | tests/libgit2/repo/open.c | 94 |
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 */ |