diff options
author | Edward Thomson <ethomson@edwardthomson.com> | 2022-07-12 14:42:50 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-07-12 14:42:50 -0400 |
commit | 4ae8704b9f4a53d4125e9195812890cbf64177c5 (patch) | |
tree | 4faad93c0dd9f8e0e14d0a111b3229456b2b2176 | |
parent | d1001fd05bd20b92b7568d93b0471feb3547035b (diff) | |
parent | ed24b8bacc739b072a789ca7396e46bd751ca2ec (diff) | |
download | libgit2-4ae8704b9f4a53d4125e9195812890cbf64177c5.tar.gz |
Merge pull request #6349 from libgit2/ethomson/cve-2022-29187
Fixes for CVE 2022-29187
-rw-r--r-- | src/libgit2/repository.c | 110 | ||||
-rw-r--r-- | src/util/fs_path.c | 51 | ||||
-rw-r--r-- | src/util/fs_path.h | 7 |
3 files changed, 122 insertions, 46 deletions
diff --git a/src/libgit2/repository.c b/src/libgit2/repository.c index 09e81cded..f761b5f32 100644 --- a/src/libgit2/repository.c +++ b/src/libgit2/repository.c @@ -488,7 +488,7 @@ static int read_gitfile(git_str *path_out, const char *file_path) typedef struct { const char *repo_path; git_str tmp; - bool is_safe; + bool *is_safe; } validate_ownership_data; static int validate_ownership_cb(const git_config_entry *entry, void *payload) @@ -496,52 +496,102 @@ static int validate_ownership_cb(const git_config_entry *entry, void *payload) validate_ownership_data *data = payload; if (strcmp(entry->value, "") == 0) - data->is_safe = false; + *data->is_safe = false; if (git_fs_path_prettify_dir(&data->tmp, entry->value, NULL) == 0 && strcmp(data->tmp.ptr, data->repo_path) == 0) - data->is_safe = true; + *data->is_safe = true; return 0; } -static int validate_ownership(const char *repo_path) +static int validate_ownership_config(bool *is_safe, const char *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; + validate_ownership_data ownership_data = { + path, GIT_STR_INIT, is_safe + }; + git_config *config; int error; - if ((error = git_fs_path_owner_is(&is_safe, repo_path, owner_level)) < 0) { - if (error == GIT_ENOTFOUND) - error = 0; + if (load_global_config(&config) != 0) + return 0; - goto done; - } + error = git_config_get_multivar_foreach(config, + "safe.directory", NULL, + validate_ownership_cb, + &ownership_data); + + git_config_free(config); + git_str_dispose(&ownership_data.tmp); - if (is_safe) { + return error; +} + +static int validate_ownership_path(bool *is_safe, const char *path) +{ + git_fs_path_owner_t owner_level = + GIT_FS_PATH_OWNER_CURRENT_USER | + GIT_FS_PATH_USER_IS_ADMINISTRATOR | + GIT_FS_PATH_OWNER_RUNNING_SUDO; + int error = 0; + + if (path) + error = git_fs_path_owner_is(is_safe, path, owner_level); + + if (error == GIT_ENOTFOUND) { + *is_safe = true; error = 0; - goto done; } - if (load_global_config(&config) == 0) { - error = git_config_get_multivar_foreach(config, "safe.directory", NULL, validate_ownership_cb, &data); + return error; +} + +static int validate_ownership(git_repository *repo) +{ + const char *validation_paths[3] = { NULL }, *path; + size_t validation_len = 0, i; + bool is_safe = false; + int error = 0; + + /* + * If there's a worktree, validate the permissions to it *and* + * the git directory, and use the worktree as the configuration + * key for allowlisting the directory. In a bare setup, only + * look at the gitdir and use that as the allowlist. So we + * examine all `validation_paths` but use only the first as + * the configuration lookup. + */ + + if (repo->workdir) + validation_paths[validation_len++] = repo->workdir; - if (!error && data.is_safe) + if (repo->gitlink) + validation_paths[validation_len++] = repo->gitlink; + + validation_paths[validation_len++] = repo->gitdir; + + for (i = 0; i < validation_len; i++) { + path = validation_paths[i]; + + if ((error = validate_ownership_path(&is_safe, path)) < 0) goto done; + + if (!is_safe) + break; } - git_error_set(GIT_ERROR_CONFIG, - "repository path '%s' is not owned by current user", - repo_path); - error = GIT_EOWNER; + if (is_safe || + (error = validate_ownership_config(&is_safe, validation_paths[0])) < 0) + goto done; + + if (!is_safe) { + git_error_set(GIT_ERROR_CONFIG, + "repository path '%s' is not owned by current user", + path); + error = GIT_EOWNER; + } done: - git_config_free(config); - git_str_dispose(&data.tmp); return error; } @@ -918,7 +968,6 @@ int git_repository_open_ext( gitlink = GIT_STR_INIT, commondir = GIT_STR_INIT; git_repository *repo = NULL; git_config *config = NULL; - const char *validation_path; int version = 0; if (flags & GIT_REPOSITORY_OPEN_FROM_ENV) @@ -977,12 +1026,11 @@ int git_repository_open_ext( } /* - * Ensure that the git directory is owned by the current user. + * Ensure that the git directory and worktree are + * owned by the current user. */ - validation_path = repo->is_bare ? repo->gitdir : repo->workdir; - if (git_repository__validate_ownership && - (error = validate_ownership(validation_path)) < 0) + (error = validate_ownership(repo)) < 0) goto cleanup; cleanup: diff --git a/src/util/fs_path.c b/src/util/fs_path.c index 1eb41ef84..6c87bfd01 100644 --- a/src/util/fs_path.c +++ b/src/util/fs_path.c @@ -1934,27 +1934,36 @@ done: #else +static int sudo_uid_lookup(uid_t *out) +{ + git_str uid_str = GIT_STR_INIT; + int64_t uid; + int error; + + if ((error = git__getenv(&uid_str, "SUDO_UID")) == 0 && + (error = git__strntol64(&uid, uid_str.ptr, uid_str.size, NULL, 10)) == 0 && + uid == (int64_t)((uid_t)uid)) { + *out = (uid_t)uid; + } + + git_str_dispose(&uid_str); + return error; +} + 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; + uid_t euid, sudo_uid; 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; + euid = geteuid(); if (p_lstat(path, &st) != 0) { if (errno == ENOENT) @@ -1964,13 +1973,27 @@ int git_fs_path_owner_is( return -1; } - for (i = 0; i < uid_count; i++) { - if (uids[i] == st.st_uid) { - *out = true; - break; - } + if ((owner_type & GIT_FS_PATH_OWNER_CURRENT_USER) != 0 && + st.st_uid == euid) { + *out = true; + return 0; + } + + if ((owner_type & GIT_FS_PATH_OWNER_ADMINISTRATOR) != 0 && + st.st_uid == 0) { + *out = true; + return 0; + } + + if ((owner_type & GIT_FS_PATH_OWNER_RUNNING_SUDO) != 0 && + euid == 0 && + sudo_uid_lookup(&sudo_uid) == 0 && + st.st_uid == sudo_uid) { + *out = true; + return 0; } + *out = false; return 0; } diff --git a/src/util/fs_path.h b/src/util/fs_path.h index fd045a313..e5ca67378 100644 --- a/src/util/fs_path.h +++ b/src/util/fs_path.h @@ -747,8 +747,13 @@ typedef enum { */ GIT_FS_PATH_USER_IS_ADMINISTRATOR = (1 << 2), + /** + * The file is owned by the current user, who is running `sudo`. + */ + GIT_FS_PATH_OWNER_RUNNING_SUDO = (1 << 3), + /** The file may be owned by another user. */ - GIT_FS_PATH_OWNER_OTHER = (1 << 3) + GIT_FS_PATH_OWNER_OTHER = (1 << 4) } git_fs_path_owner_t; /** |