Origin: https://github.com/git/git/commit/3b0bf2704980b1ed6018622bdf5377ec22289688 Origin: https://github.com/git/git/commit/5f1a3fec8c304decaa9af2bf503712050a4a84e0 Origin: https://github.com/git/git/commit/ae9abbb63eea74441e3e8b153dc6ec1f94c373b4 Origin: https://github.com/git/git/commit/b9063afda17a2aa6310423c9f7b776c41f753091 Origin: https://github.com/git/git/commit/6b11e3d52e919cce91011f4f9025e6f4b61375f2 Reviewed-by: Aron Xu Last-Updated: 2023-01-26 diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt index 6d764fe..74627c5 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 63ba89d..f505f81 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -393,12 +393,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 aad9ace..9dcecda 100644 --- a/setup.c +++ b/setup.c @@ -1054,14 +1054,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; @@ -1149,6 +1167,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, '/'); @@ -1159,21 +1179,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; @@ -1306,7 +1355,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 0000000..b4d7788 --- /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 0000000..ff31176 --- /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