diff --git a/Documentation/RelNotes/2.30.5.txt b/Documentation/RelNotes/2.30.5.txt new file mode 100644 index 0000000000..5191cab3ae --- /dev/null +++ b/Documentation/RelNotes/2.30.5.txt @@ -0,0 +1,12 @@ +Git v2.30.5 Release Notes +========================= + +This release contains minor fix-ups for the changes that went into +Git 2.30.3 and 2.30.4, addressing CVE-2022-29187. + + * The safety check that verifies a safe ownership of the Git + worktree is now extended to also cover the ownership of the Git + directory (and the `.git` file, if there is any). + +Carlo Marcelo Arenas Belón (1): + setup: tighten ownership checks post CVE-2022-24765 diff --git a/Documentation/RelNotes/2.31.4.txt b/Documentation/RelNotes/2.31.4.txt new file mode 100644 index 0000000000..97a91fd07a --- /dev/null +++ b/Documentation/RelNotes/2.31.4.txt @@ -0,0 +1,6 @@ +Git v2.31.4 Release Notes +========================= + +This release merges up the fixes that appear in v2.30.5 to address +the security issue CVE-2022-29187; see the release notes for that +version for details. diff --git a/Documentation/RelNotes/2.32.3.txt b/Documentation/RelNotes/2.32.3.txt new file mode 100644 index 0000000000..583fabe684 --- /dev/null +++ b/Documentation/RelNotes/2.32.3.txt @@ -0,0 +1,6 @@ +Git v2.32.3 Release Notes +========================= + +This release merges up the fixes that appear in v2.30.5 and +v2.31.4 to address the security issue CVE-2022-29187; see the +release notes for these versions for details. diff --git a/Documentation/RelNotes/2.33.4.txt b/Documentation/RelNotes/2.33.4.txt new file mode 100644 index 0000000000..a145cc25de --- /dev/null +++ b/Documentation/RelNotes/2.33.4.txt @@ -0,0 +1,6 @@ +Git v2.33.4 Release Notes +========================= + +This release merges up the fixes that appear in v2.30.5, v2.31.4 +and v2.32.3 to address the security issue CVE-2022-29187; see +the release notes for these versions for details. diff --git a/Documentation/RelNotes/2.34.4.txt b/Documentation/RelNotes/2.34.4.txt new file mode 100644 index 0000000000..2a6b223403 --- /dev/null +++ b/Documentation/RelNotes/2.34.4.txt @@ -0,0 +1,6 @@ +Git v2.34.4 Release Notes +========================= + +This release merges up the fixes that appear in v2.30.5, v2.31.4, +v2.32.3 and v2.33.4 to address the security issue CVE-2022-29187; +see the release notes for these versions for details. diff --git a/Documentation/RelNotes/2.35.4.txt b/Documentation/RelNotes/2.35.4.txt new file mode 100644 index 0000000000..47abd5ad45 --- /dev/null +++ b/Documentation/RelNotes/2.35.4.txt @@ -0,0 +1,7 @@ +Git v2.35.4 Release Notes +========================= + +This release merges up the fixes that appear in v2.30.5, +v2.31.4, v2.32.3, v2.33.4 and v2.34.4 to address the security +issue CVE-2022-29187; see the release notes for these versions +for details. diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt index 6d764fe0cc..74627c5e7c 100644 --- a/Documentation/config/safe.txt +++ b/Documentation/config/safe.txt @@ -26,3 +26,17 @@ directory was listed in the `safe.directory` list. If `safe.directory=*` is set in system config and you want to re-enable this protection, then initialize your list with an empty value before listing the repositories that you deem safe. ++ +As explained, Git only allows you to access repositories owned by +yourself, i.e. the user who is running Git, by default. When Git +is running as 'root' in a non Windows platform that provides sudo, +however, git checks the SUDO_UID environment variable that sudo creates +and will allow access to the uid recorded as its value in addition to +the id from 'root'. +This is to make it easy to perform a common sequence during installation +"make && sudo make install". A git process running under 'sudo' runs as +'root' but the 'sudo' command exports the environment variable to record +which id the original user has. +If that is not what you would prefer and want git to only trust +repositories that are owned by root instead, then you can remove +the `SUDO_UID` variable from root's environment before invoking git. diff --git a/git-compat-util.h b/git-compat-util.h index 58fd813bd0..ccf06289c3 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -437,12 +437,68 @@ static inline int git_offset_1st_component(const char *path) #endif #ifndef is_path_owned_by_current_user + +#ifdef __TANDEM +#define ROOT_UID 65535 +#else +#define ROOT_UID 0 +#endif + +/* + * Do not use this function when + * (1) geteuid() did not say we are running as 'root', or + * (2) using this function will compromise the system. + * + * PORTABILITY WARNING: + * This code assumes uid_t is unsigned because that is what sudo does. + * If your uid_t type is signed and all your ids are positive then it + * should all work fine. + * If your version of sudo uses negative values for uid_t or it is + * buggy and return an overflowed value in SUDO_UID, then git might + * fail to grant access to your repository properly or even mistakenly + * grant access to someone else. + * In the unlikely scenario this happened to you, and that is how you + * got to this message, we would like to know about it; so sent us an + * email to git@vger.kernel.org indicating which platform you are + * using and which version of sudo, so we can improve this logic and + * maybe provide you with a patch that would prevent this issue again + * in the future. + */ +static inline void extract_id_from_env(const char *env, uid_t *id) +{ + const char *real_uid = getenv(env); + + /* discard anything empty to avoid a more complex check below */ + if (real_uid && *real_uid) { + char *endptr = NULL; + unsigned long env_id; + + errno = 0; + /* silent overflow errors could trigger a bug here */ + env_id = strtoul(real_uid, &endptr, 10); + if (!*endptr && !errno) + *id = env_id; + } +} + static inline int is_path_owned_by_current_uid(const char *path) { struct stat st; + uid_t euid; + if (lstat(path, &st)) return 0; - return st.st_uid == geteuid(); + + euid = geteuid(); + if (euid == ROOT_UID) + { + if (st.st_uid == ROOT_UID) + return 1; + else + extract_id_from_env("SUDO_UID", &euid); + } + + return st.st_uid == euid; } #define is_path_owned_by_current_user is_path_owned_by_current_uid diff --git a/setup.c b/setup.c index a7b36f3ffb..ca6a5ddb91 100644 --- a/setup.c +++ b/setup.c @@ -1120,14 +1120,32 @@ static int safe_directory_cb(const char *key, const char *value, void *d) return 0; } -static int ensure_valid_ownership(const char *path) +/* + * Check if a repository is safe, by verifying the ownership of the + * worktree (if any), the git directory, and the gitfile (if any). + * + * Exemptions for known-safe repositories can be added via `safe.directory` + * config settings; for non-bare repositories, their worktree needs to be + * added, for bare ones their git directory. + */ +static int ensure_valid_ownership(const char *gitfile, + const char *worktree, const char *gitdir) { - struct safe_directory_data data = { .path = path }; + struct safe_directory_data data = { + .path = worktree ? worktree : gitdir + }; if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) && - is_path_owned_by_current_user(path)) + (!gitfile || is_path_owned_by_current_user(gitfile)) && + (!worktree || is_path_owned_by_current_user(worktree)) && + (!gitdir || is_path_owned_by_current_user(gitdir))) return 1; + /* + * data.path is the "path" that identifies the repository and it is + * constant regardless of what failed above. data.is_safe should be + * initialized to false, and might be changed by the callback. + */ read_very_early_config(safe_directory_cb, &data); return data.is_safe; @@ -1215,6 +1233,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, current_device = get_device_or_die(dir->buf, NULL, 0); for (;;) { int offset = dir->len, error_code = 0; + char *gitdir_path = NULL; + char *gitfile = NULL; if (offset > min_offset) strbuf_addch(dir, '/'); @@ -1225,21 +1245,50 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, if (die_on_error || error_code == READ_GITFILE_ERR_NOT_A_FILE) { /* NEEDSWORK: fail if .git is not file nor dir */ - if (is_git_directory(dir->buf)) + if (is_git_directory(dir->buf)) { gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT; + gitdir_path = xstrdup(dir->buf); + } } else if (error_code != READ_GITFILE_ERR_STAT_FAILED) return GIT_DIR_INVALID_GITFILE; - } + } else + gitfile = xstrdup(dir->buf); + /* + * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT + * to check that directory for a repository. + * Now trim that tentative addition away, because we want to + * focus on the real directory we are in. + */ strbuf_setlen(dir, offset); if (gitdirenv) { - if (!ensure_valid_ownership(dir->buf)) - return GIT_DIR_INVALID_OWNERSHIP; - strbuf_addstr(gitdir, gitdirenv); - return GIT_DIR_DISCOVERED; + enum discovery_result ret; + + if (ensure_valid_ownership(gitfile, + dir->buf, + (gitdir_path ? gitdir_path : gitdirenv))) { + strbuf_addstr(gitdir, gitdirenv); + ret = GIT_DIR_DISCOVERED; + } else + ret = GIT_DIR_INVALID_OWNERSHIP; + + /* + * Earlier, during discovery, we might have allocated + * string copies for gitdir_path or gitfile so make + * sure we don't leak by freeing them now, before + * leaving the loop and function. + * + * Note: gitdirenv will be non-NULL whenever these are + * allocated, therefore we need not take care of releasing + * them outside of this conditional block. + */ + free(gitdir_path); + free(gitfile); + + return ret; } if (is_git_directory(dir->buf)) { - if (!ensure_valid_ownership(dir->buf)) + if (!ensure_valid_ownership(NULL, NULL, dir->buf)) return GIT_DIR_INVALID_OWNERSHIP; strbuf_addstr(gitdir, "."); return GIT_DIR_BARE; @@ -1377,7 +1426,7 @@ const char *setup_git_directory_gently(int *nongit_ok) struct strbuf quoted = STRBUF_INIT; sq_quote_buf_pretty("ed, dir.buf); - die(_("unsafe repository ('%s' is owned by someone else)\n" + die(_("detected dubious ownership in repository at '%s'\n" "To add an exception for this directory, call:\n" "\n" "\tgit config --global --add safe.directory %s"), diff --git a/t/lib-sudo.sh b/t/lib-sudo.sh new file mode 100644 index 0000000000..b4d7788f4e --- /dev/null +++ b/t/lib-sudo.sh @@ -0,0 +1,15 @@ +# Helpers for running git commands under sudo. + +# Runs a scriplet passed through stdin under sudo. +run_with_sudo () { + local ret + local RUN="$TEST_DIRECTORY/$$.sh" + write_script "$RUN" "$TEST_SHELL_PATH" + # avoid calling "$RUN" directly so sudo doesn't get a chance to + # override the shell, add aditional restrictions or even reject + # running the script because its security policy deem it unsafe + sudo "$TEST_SHELL_PATH" -c "\"$RUN\"" + ret=$? + rm -f "$RUN" + return $ret +} diff --git a/t/t0034-root-safe-directory.sh b/t/t0034-root-safe-directory.sh new file mode 100755 index 0000000000..ff31176128 --- /dev/null +++ b/t/t0034-root-safe-directory.sh @@ -0,0 +1,93 @@ +#!/bin/sh + +test_description='verify safe.directory checks while running as root' + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-sudo.sh + +if [ "$GIT_TEST_ALLOW_SUDO" != "YES" ] +then + skip_all="You must set env var GIT_TEST_ALLOW_SUDO=YES in order to run this test" + test_done +fi + +if ! test_have_prereq NOT_ROOT +then + skip_all="These tests do not support running as root" + test_done +fi + +test_lazy_prereq SUDO ' + sudo -n id -u >u && + id -u root >r && + test_cmp u r && + command -v git >u && + sudo command -v git >r && + test_cmp u r +' + +if ! test_have_prereq SUDO +then + skip_all="Your sudo/system configuration is either too strict or unsupported" + test_done +fi + +test_expect_success SUDO 'setup' ' + sudo rm -rf root && + mkdir -p root/r && + ( + cd root/r && + git init + ) +' + +test_expect_success SUDO 'sudo git status as original owner' ' + ( + cd root/r && + git status && + sudo git status + ) +' + +test_expect_success SUDO 'setup root owned repository' ' + sudo mkdir -p root/p && + sudo git init root/p +' + +test_expect_success 'cannot access if owned by root' ' + ( + cd root/p && + test_must_fail git status + ) +' + +test_expect_success 'can access if addressed explicitly' ' + ( + cd root/p && + GIT_DIR=.git GIT_WORK_TREE=. git status + ) +' + +test_expect_success SUDO 'can access with sudo if root' ' + ( + cd root/p && + sudo git status + ) +' + +test_expect_success SUDO 'can access with sudo if root by removing SUDO_UID' ' + ( + cd root/p && + run_with_sudo <<-END + unset SUDO_UID && + git status + END + ) +' + +# this MUST be always the last test +test_expect_success SUDO 'cleanup' ' + sudo rm -rf root +' + +test_done