Sync with 2.33.4

dscho · dscho · commit 378eaded1aec · 2022-06-23T12:35:47.000+02:00
* 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -401,12 +401,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
@@ -1055,14 +1055,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;
@@ -1150,6 +1168,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, '/');
@@ -1160,21 +1180,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;
@@ -1312,7 +1361,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			struct strbuf quoted = STRBUF_INIT;
 
 			sq_quote_buf_pretty(&quoted, 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
@@ -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
@@ -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
