Unverified Commit d1001fd0 by Edward Thomson Committed by GitHub

Merge pull request #6341 from libgit2/ethomson/ownership2

Fix erroneously lax configuration ownership checks
parents 92ffdd2c 56aaaf53
...@@ -1170,10 +1170,13 @@ int git_config_find_programdata(git_buf *path) ...@@ -1170,10 +1170,13 @@ int git_config_find_programdata(git_buf *path)
int git_config__find_programdata(git_str *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; bool is_safe;
if (git_sysdir_find_programdata_file(path, GIT_CONFIG_FILENAME_PROGRAMDATA) < 0 || 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; return -1;
if (!is_safe) { if (!is_safe) {
......
...@@ -509,10 +509,13 @@ static int validate_ownership(const char *repo_path) ...@@ -509,10 +509,13 @@ static int validate_ownership(const char *repo_path)
{ {
git_config *config = NULL; git_config *config = NULL;
validate_ownership_data data = { repo_path, GIT_STR_INIT, false }; 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; bool is_safe;
int error; 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) if (error == GIT_ENOTFOUND)
error = 0; error = 0;
......
...@@ -1785,9 +1785,9 @@ done: ...@@ -1785,9 +1785,9 @@ done:
return supported; 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; mock_owner = owner;
} }
...@@ -1879,74 +1879,52 @@ static int file_owner_sid(PSID *out, const char *path) ...@@ -1879,74 +1879,52 @@ static int file_owner_sid(PSID *out, const char *path)
return error; 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; PSID owner_sid = NULL, user_sid = NULL;
int error = -1; BOOL is_admin, admin_owned;
int error;
if (mock_owner) { if (mock_owner) {
*out = (mock_owner == GIT_FS_PATH_MOCK_OWNER_CURRENT_USER); *out = ((mock_owner & owner_type) != 0);
return 0; return 0;
} }
if ((error = file_owner_sid(&owner_sid, path)) < 0 || if ((error = file_owner_sid(&owner_sid, path)) < 0)
(error = current_user_sid(&user_sid)) < 0)
goto done; goto done;
*out = EqualSid(owner_sid, user_sid); if ((owner_type & GIT_FS_PATH_OWNER_CURRENT_USER) != 0) {
error = 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) { if (EqualSid(owner_sid, user_sid)) {
*out = (mock_owner == GIT_FS_PATH_MOCK_OWNER_SYSTEM); *out = true;
return 0; goto done;
} }
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 (file_owner_sid(&owner_sid, path) < 0) admin_owned =
goto done; IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) ||
IsWellKnownSid(owner_sid, WinLocalSystemSid);
if (IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) || if (admin_owned &&
IsWellKnownSid(owner_sid, WinLocalSystemSid)) { (owner_type & GIT_FS_PATH_OWNER_ADMINISTRATOR) != 0) {
*out = 1; *out = true;
error = 0;
goto done; 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; goto done;
}
*out = EqualSid(owner_sid, user_sid); *out = false;
error = 0;
done: done:
git__free(owner_sid); git__free(owner_sid);
...@@ -1956,10 +1934,25 @@ done: ...@@ -1956,10 +1934,25 @@ done:
#else #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; 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; *out = false;
...@@ -1971,7 +1964,7 @@ static int fs_path_owner_is(bool *out, const char *path, uid_t *uids, size_t uid ...@@ -1971,7 +1964,7 @@ static int fs_path_owner_is(bool *out, const char *path, uid_t *uids, size_t uid
return -1; return -1;
} }
for (i = 0; i < uids_len; i++) { for (i = 0; i < uid_count; i++) {
if (uids[i] == st.st_uid) { if (uids[i] == st.st_uid) {
*out = true; *out = true;
break; break;
...@@ -1981,45 +1974,18 @@ static int fs_path_owner_is(bool *out, const char *path, uid_t *uids, size_t uid ...@@ -1981,45 +1974,18 @@ static int fs_path_owner_is(bool *out, const char *path, uid_t *uids, size_t uid
return 0; return 0;
} }
#endif
int git_fs_path_owner_is_current_user(bool *out, const char *path) int git_fs_path_owner_is_current_user(bool *out, const char *path)
{ {
uid_t userid = geteuid(); return git_fs_path_owner_is(out, path, GIT_FS_PATH_OWNER_CURRENT_USER);
if (mock_owner) {
*out = (mock_owner == GIT_FS_PATH_MOCK_OWNER_CURRENT_USER);
return 0;
}
return fs_path_owner_is(out, path, &userid, 1);
} }
int git_fs_path_owner_is_system(bool *out, const char *path) int git_fs_path_owner_is_system(bool *out, const char *path)
{ {
uid_t userid = 0; return git_fs_path_owner_is(out, path, GIT_FS_PATH_OWNER_ADMINISTRATOR);
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);
} }
#endif
int git_fs_path_find_executable(git_str *fullpath, const char *executable) int git_fs_path_find_executable(git_str *fullpath, const char *executable)
{ {
#ifdef GIT_WIN32 #ifdef GIT_WIN32
......
...@@ -732,18 +732,37 @@ int git_fs_path_normalize_slashes(git_str *out, const char *path); ...@@ -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); bool git_fs_path_supports_symlinks(const char *dir);
typedef enum { typedef enum {
GIT_FS_PATH_MOCK_OWNER_NONE = 0, /* do filesystem lookups as normal */ GIT_FS_PATH_OWNER_NONE = 0,
GIT_FS_PATH_MOCK_OWNER_SYSTEM = 1,
GIT_FS_PATH_MOCK_OWNER_CURRENT_USER = 2, /** The file must be owned by the current user. */
GIT_FS_PATH_MOCK_OWNER_OTHER = 3 GIT_FS_PATH_OWNER_CURRENT_USER = (1 << 0),
} git_fs_path__mock_owner_t;
/** 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 * Sets the mock ownership for files; subsequent calls to
* `git_fs_path_owner_is_*` functions will return this data until cleared * `git_fs_path_owner_is_*` functions will return this data until
* with `GIT_FS_PATH_MOCK_OWNER_NONE`. * 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 * 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); ...@@ -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); 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 * Search the current PATH for the given executable, returning the full
* path if it is found. * path if it is found.
*/ */
......
...@@ -16,12 +16,13 @@ void test_repo_open__cleanup(void) ...@@ -16,12 +16,13 @@ void test_repo_open__cleanup(void)
{ {
cl_git_sandbox_cleanup(); cl_git_sandbox_cleanup();
cl_fixture_cleanup("empty_standard_repo"); cl_fixture_cleanup("empty_standard_repo");
cl_fixture_cleanup("testrepo.git");
cl_fixture_cleanup("__global_config"); cl_fixture_cleanup("__global_config");
if (git_fs_path_isdir("alternate")) if (git_fs_path_isdir("alternate"))
git_futils_rmdir_r("alternate", NULL, GIT_RMDIR_REMOVE_FILES); 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)); cl_git_pass(git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, config_path.ptr));
git_buf_dispose(&config_path); git_buf_dispose(&config_path);
...@@ -480,20 +481,55 @@ void test_repo_open__validates_dir_ownership(void) ...@@ -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")); cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git"));
/* When the current user owns the repo config, that's acceptable */ /* 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")); cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
git_repository_free(repo); git_repository_free(repo);
/* When the system user owns the repo config, also acceptable */ /* When the system user owns the repo config, fail */
git_fs_path__set_owner(GIT_FS_PATH_MOCK_OWNER_SYSTEM); 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")); cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
git_repository_free(repo); git_repository_free(repo);
#endif
/* When an unknown user owns the repo config, fail */ /* 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")); 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) void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void)
{ {
git_repository *repo; git_repository *repo;
...@@ -506,7 +542,7 @@ void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void) ...@@ -506,7 +542,7 @@ void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void)
cl_fixture_sandbox("empty_standard_repo"); cl_fixture_sandbox("empty_standard_repo");
cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git")); 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")); cl_git_fail(git_repository_open(&repo, "empty_standard_repo"));
/* Add safe.directory options to the global configuration */ /* Add safe.directory options to the global configuration */
...@@ -539,6 +575,50 @@ void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void) ...@@ -539,6 +575,50 @@ void test_repo_open__can_allowlist_dirs_with_problematic_ownership(void)
git_str_dispose(&config_data); 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) void test_repo_open__can_reset_safe_directory_list(void)
{ {
git_repository *repo; git_repository *repo;
...@@ -551,7 +631,7 @@ void test_repo_open__can_reset_safe_directory_list(void) ...@@ -551,7 +631,7 @@ void test_repo_open__can_reset_safe_directory_list(void)
cl_fixture_sandbox("empty_standard_repo"); cl_fixture_sandbox("empty_standard_repo");
cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git")); 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")); cl_git_fail(git_repository_open(&repo, "empty_standard_repo"));
/* Add safe.directory options to the global configuration */ /* Add safe.directory options to the global configuration */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment