Sync with 2.33.4
* maint-2.33:
Git 2.33.4
Git 2.32.3
Git 2.31.4
Git 2.30.5
setup: tighten ownership checks post CVE-2022-24765
git-compat-util: allow root to access both SUDO_UID and root owned
t0034: add negative tests and allow git init to mostly work under sudo
git-compat-util: avoid failing dir ownership checks if running privileged
t: regression git needs safe.directory when using sudo
diff --git a/Documentation/RelNotes/2.30.5.txt b/Documentation/RelNotes/2.30.5.txt
new file mode 100644
index 0000000..5191cab
--- /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 0000000..97a91fd
--- /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 0000000..583fabe
--- /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 0000000..a145cc2
--- /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/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 @@
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 4b57ae6..6cef176 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -401,12 +401,68 @@
#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 a9161ac..114d9ee 100644
--- a/setup.c
+++ b/setup.c
@@ -1055,14 +1055,32 @@
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;
@@ -1150,6 +1168,8 @@
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, '/');
@@ -1160,21 +1180,50 @@
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;
@@ -1312,7 +1361,7 @@
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
