CVE-2024-32020: Insufficient Information (NVD-CWE-noinfo)
value: Git is a revision control system. Prior to versions 2.45.1, 2.44.1, 2.43.4, 2.42.2, 2.41.1, 2.40.2, and 2.39.4, local clones may end up hardlinking files into the target repository's object database when source and target repository reside on the same disk. If the source repository is owned by a different user, then those hardlinked files may be rewritten at any point in time by the untrusted user. Cloning local repositories will cause Git to either copy or hardlink files of the source repository into the target repository. This significantly speeds up such local clones compared to doing a "proper" clone and saves both disk space and compute time. When cloning a repository located on the same disk that is owned by a different user than the current user we also end up creating such hardlinks. These files will continue to be owned and controlled by the potentially-untrusted user and can be rewritten by them at will in the future. The problem has been patched in versions 2.45.1, 2.44.1, 2.43.4, 2.42.2, 2.41.1, 2.40.2, and 2.39.4.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-05-14T19:15Z
Repository: https://github.com/git/git
Language: C
Files changed: builtin/clone.c, t/t0033-safe-directory.sh
Commit date: 2024-04-17 02:17:40 +0200
Commit message: builtin/clone: refuse local clones of unsafe repositories

When performing a local clone of a repository we end up either copying
or hardlinking the source repository into the target repository. This is
significantly more performant than if we were to use git-upload-pack(1)
and git-fetch-pack(1) to create the new repository and preserves both
disk space and compute time.

Unfortunately though, performing such a local clone of a repository that
is not owned by the current user is inherently unsafe:

  - It is possible that source files get swapped out underneath us while
    we are copying or hardlinking them. While we do perform some checks
    here to assert that we hardlinked the expected file, they cannot
    reliably thwart time-of-check-time-of-use (TOCTOU) style races. It
    is thus possible for an adversary to make us copy or hardlink
    unexpected files into the target directory.

    Ideally, we would address this by starting to use openat(3P),
    fstatat(3P) and friends. Due to platform compatibility with Windows
    we cannot easily do that though. Furthermore, the scope of these
    fixes would likely be quite broad and thus not fit for an embargoed
    security release.

  - Even if we handled TOCTOU-style races perfectly, hardlinking files
    owned by a different user into the target repository is not a good
    idea in general. It is possible for an adversary to rewrite those
    files to contain whatever data they want even after the clone has
    completed.

Address these issues by completely refusing local clones of a repository
that is not owned by the current user. This reuses our existing infra we
have in place via `ensure_valid_ownership()` and thus allows a user to
override the safety guard by adding the source repository path to the
"safe.directory" configuration.

This addresses CVE-2024-32020.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Diff: diff --git a/builtin/clone.c b/builtin/clone.c
index 4b80fa0870b..9ec500d427e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -319,10 +319,24 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 {
 	int src_len, dest_len;
 	struct dir_iterator *iter;
 	int iter_status;
 
+	/*
+	 * Refuse copying directories by default which aren't owned by us. The
+	 * code that performs either the copying or hardlinking is not prepared
+	 * to handle various edge cases where an adversary may for example
+	 * racily swap out files for symlinks. This can cause us to
+	 * inadvertently use the wrong source file.
+	 *
+	 * Furthermore, even if we were prepared to handle such races safely,
+	 * creating hardlinks across user boundaries is an inherently unsafe
+	 * operation as the hardlinked files can be rewritten at will by the
+	 * potentially-untrusted user. We thus refuse to do so by default.
+	 */
+	die_upon_dubious_ownership(NULL, NULL, src_repo);
+
 	mkdir_if_missing(dest->buf, 0777);
 
 	iter = dir_iterator_begin(src->buf, DIR_ITERATOR_PEDANTIC);
 
 	if (!iter)
diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
index dc3496897ab..11c3e8f28e0 100755
--- a/t/t0033-safe-directory.sh
+++ b/t/t0033-safe-directory.sh
@@ -78,6 +78,30 @@ test_expect_success 'safe.directory in included file' '
 	EOF
 	git config --global --add include.path "$(pwd)/gitconfig-include" &&
 	git status
 '
 
+test_expect_success 'local clone of unowned repo refused in unsafe directory' '
+	test_when_finished "rm -rf source" &&
+	git init source &&
+	(
+		sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+		test_commit -C source initial
+	) &&
+	test_must_fail git clone --local source target &&
+	test_path_is_missing target
+'
+
+test_expect_success 'local clone of unowned repo accepted in safe directory' '
+	test_when_finished "rm -rf source" &&
+	git init source &&
+	(
+		sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+		test_commit -C source initial
+	) &&
+	test_must_fail git clone --local source target &&
+	git config --global --add safe.directory "$(pwd)/source/.git" &&
+	git clone --local source target &&
+	test_path_is_dir target
+'
+
 test_done

Fixed code: 	/*
	 * Refuse copying directories by default which aren't owned by us. The
	 * code that performs either the copying or hardlinking is not prepared
	 * to handle various edge cases where an adversary may for example
	 * racily swap out files for symlinks. This can cause us to
	 * inadvertently use the wrong source file.
	 *
	 * Furthermore, even if we were prepared to handle such races safely,
	 * creating hardlinks across user boundaries is an inherently unsafe
	 * operation as the hardlinked files can be rewritten at will by the
	 * potentially-untrusted user. We thus refuse to do so by default.
	 */
	die_upon_dubious_ownership(NULL, NULL, src_repo);

test_expect_success 'local clone of unowned repo refused in unsafe directory' '
	test_when_finished "rm -rf source" &&
	git init source &&
	(
		sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
		test_commit -C source initial
	) &&
	test_must_fail git clone --local source target &&
	test_path_is_missing target
'

test_expect_success 'local clone of unowned repo accepted in safe directory' '
	test_when_finished "rm -rf source" &&
	git init source &&
	(
		sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
		test_commit -C source initial
	) &&
	test_must_fail git clone --local source target &&
	git config --global --add safe.directory "$(pwd)/source/.git" &&
	git clone --local source target &&
	test_path_is_dir target
'