diff --git a/backport-CVE-2024-32004-fetch-clone-detect-dubious-ownership-of-local-reposi.patch b/backport-CVE-2024-32004-fetch-clone-detect-dubious-ownership-of-local-reposi.patch new file mode 100644 index 0000000000000000000000000000000000000000..271da261fca1bcac31098a4a035f5da7ad7c220e --- /dev/null +++ b/backport-CVE-2024-32004-fetch-clone-detect-dubious-ownership-of-local-reposi.patch @@ -0,0 +1,153 @@ +From f4aa8c8bb11dae6e769cd930565173808cbb69c8 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Wed, 10 Apr 2024 14:39:37 +0200 +Subject: [PATCH] fetch/clone: detect dubious ownership of local repositories + +When cloning from somebody else's repositories, it is possible that, +say, the `upload-pack` command is overridden in the repository that is +about to be cloned, which would then be run in the user's context who +started the clone. + +To remind the user that this is a potentially unsafe operation, let's +extend the ownership checks we have already established for regular +gitdir discovery to extend also to local repositories that are about to +be cloned. + +This protection extends also to file:// URLs. + +The fixes in this commit address CVE-2024-32004. + +Note: This commit does not touch the `fetch`/`clone` code directly, but +instead the function used implicitly by both: `enter_repo()`. This +function is also used by `git receive-pack` (i.e. pushes), by `git +upload-archive`, by `git daemon` and by `git http-backend`. In setups +that want to serve repositories owned by different users than the +account running the service, this will require `safe.*` settings to be +configured accordingly. + +Also note: there are tiny time windows where a time-of-check-time-of-use +("TOCTOU") race is possible. The real solution to those would be to work +with `fstat()` and `openat()`. However, the latter function is not +available on Windows (and would have to be emulated with rather +expensive low-level `NtCreateFile()` calls), and the changes would be +quite extensive, for my taste too extensive for the little gain given +that embargoed releases need to pay extra attention to avoid introducing +inadvertent bugs. + +Signed-off-by: Johannes Schindelin +--- + setup.h | 12 ++++++++++++ + path.c | 2 ++ + setup.c | 21 +++++++++++++++++++++ + t/t0411-clone-from-partial.sh | 6 +++--- + 4 files changed, 38 insertions(+), 3 deletions(-) + +diff --git a/setup.h b/setup.h +index fcf49706a..a46a3e4b6 100644 +--- a/setup.h ++++ b/setup.h +@@ -41,6 +41,18 @@ const char *read_gitfile_gently(const char *path, int *return_error_code); + const char *resolve_gitdir_gently(const char *suspect, int *return_error_code); + #define resolve_gitdir(path) resolve_gitdir_gently((path), NULL) + ++/* ++ * Check if a repository is safe and die if it is not, 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. ++ */ ++void die_upon_dubious_ownership(const char *gitfile, const char *worktree, ++ const char *gitdir); ++ + void setup_work_tree(void); + + /* +diff --git a/path.c b/path.c +index 492e17ad1..d61f70e87 100644 +--- a/path.c ++++ b/path.c +@@ -840,6 +840,7 @@ const char *enter_repo(const char *path, int strict) + if (!suffix[i]) + return NULL; + gitfile = read_gitfile(used_path.buf); ++ die_upon_dubious_ownership(gitfile, NULL, used_path.buf); + if (gitfile) { + strbuf_reset(&used_path); + strbuf_addstr(&used_path, gitfile); +@@ -850,6 +851,7 @@ const char *enter_repo(const char *path, int strict) + } + else { + const char *gitfile = read_gitfile(path); ++ die_upon_dubious_ownership(gitfile, NULL, path); + if (gitfile) + path = gitfile; + if (chdir(path)) +diff --git a/setup.c b/setup.c +index cefd5f63c..9d401ae4c 100644 +--- a/setup.c ++++ b/setup.c +@@ -1165,6 +1165,27 @@ static int ensure_valid_ownership(const char *gitfile, + return data.is_safe; + } + ++void die_upon_dubious_ownership(const char *gitfile, const char *worktree, ++ const char *gitdir) ++{ ++ struct strbuf report = STRBUF_INIT, quoted = STRBUF_INIT; ++ const char *path; ++ ++ if (ensure_valid_ownership(gitfile, worktree, gitdir, &report)) ++ return; ++ ++ strbuf_complete(&report, '\n'); ++ path = gitfile ? gitfile : gitdir; ++ sq_quote_buf_pretty("ed, path); ++ ++ die(_("detected dubious ownership in repository at '%s'\n" ++ "%s" ++ "To add an exception for this directory, call:\n" ++ "\n" ++ "\tgit config --global --add safe.directory %s"), ++ path, report.buf, quoted.buf); ++} ++ + static int allowed_bare_repo_cb(const char *key, const char *value, + const struct config_context *ctx UNUSED, + void *d) +diff --git a/t/t0411-clone-from-partial.sh b/t/t0411-clone-from-partial.sh +index fb72a0a9f..eb3360dbc 100755 +--- a/t/t0411-clone-from-partial.sh ++++ b/t/t0411-clone-from-partial.sh +@@ -23,7 +23,7 @@ test_expect_success 'create evil repo' ' + >evil/.git/shallow + ' + +-test_expect_failure 'local clone must not fetch from promisor remote and execute script' ' ++test_expect_success 'local clone must not fetch from promisor remote and execute script' ' + rm -f script-executed && + test_must_fail git clone \ + --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ +@@ -32,7 +32,7 @@ test_expect_failure 'local clone must not fetch from promisor remote and execute + test_path_is_missing script-executed + ' + +-test_expect_failure 'clone from file://... must not fetch from promisor remote and execute script' ' ++test_expect_success 'clone from file://... must not fetch from promisor remote and execute script' ' + rm -f script-executed && + test_must_fail git clone \ + --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ +@@ -41,7 +41,7 @@ test_expect_failure 'clone from file://... must not fetch from promisor remote a + test_path_is_missing script-executed + ' + +-test_expect_failure 'fetch from file://... must not fetch from promisor remote and execute script' ' ++test_expect_success 'fetch from file://... must not fetch from promisor remote and execute script' ' + rm -f script-executed && + test_must_fail git fetch \ + --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ +-- +2.33.0 + diff --git a/backport-CVE-2024-32004-t0411-add-tests-for-cloning-from-partial-repo.patch b/backport-CVE-2024-32004-t0411-add-tests-for-cloning-from-partial-repo.patch new file mode 100644 index 0000000000000000000000000000000000000000..905f71d484c90d756b45df1025be0a7c7fe472b8 --- /dev/null +++ b/backport-CVE-2024-32004-t0411-add-tests-for-cloning-from-partial-repo.patch @@ -0,0 +1,90 @@ +From 5c5a4a1c05932378d259b1fdd9526cab971656a2 Mon Sep 17 00:00:00 2001 +From: Filip Hejsek +Date: Sun, 28 Jan 2024 04:29:33 +0100 +Subject: [PATCH] t0411: add tests for cloning from partial repo + +Cloning from a partial repository must not fetch missing objects into +the partial repository, because that can lead to arbitrary code +execution. + +Add a couple of test cases, pretending to the `upload-pack` command (and +to that command only) that it is working on a repository owned by +someone else. + +Helped-by: Jeff King +Signed-off-by: Filip Hejsek +Signed-off-by: Johannes Schindelin +--- + t/t0411-clone-from-partial.sh | 60 +++++++++++++++++++++++++++++++++++ + 1 file changed, 60 insertions(+) + create mode 100755 t/t0411-clone-from-partial.sh + +diff --git a/t/t0411-clone-from-partial.sh b/t/t0411-clone-from-partial.sh +new file mode 100755 +index 000000000..fb72a0a9f +--- /dev/null ++++ b/t/t0411-clone-from-partial.sh +@@ -0,0 +1,60 @@ ++#!/bin/sh ++ ++test_description='check that local clone does not fetch from promisor remotes' ++ ++. ./test-lib.sh ++ ++test_expect_success 'create evil repo' ' ++ git init tmp && ++ test_commit -C tmp a && ++ git -C tmp config uploadpack.allowfilter 1 && ++ git clone --filter=blob:none --no-local --no-checkout tmp evil && ++ rm -rf tmp && ++ ++ git -C evil config remote.origin.uploadpack \"\$TRASH_DIRECTORY/fake-upload-pack\" && ++ write_script fake-upload-pack <<-\EOF && ++ echo >&2 "fake-upload-pack running" ++ >"$TRASH_DIRECTORY/script-executed" ++ exit 1 ++ EOF ++ export TRASH_DIRECTORY && ++ ++ # empty shallow file disables local clone optimization ++ >evil/.git/shallow ++' ++ ++test_expect_failure 'local clone must not fetch from promisor remote and execute script' ' ++ rm -f script-executed && ++ test_must_fail git clone \ ++ --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ ++ evil clone1 2>err && ++ ! grep "fake-upload-pack running" err && ++ test_path_is_missing script-executed ++' ++ ++test_expect_failure 'clone from file://... must not fetch from promisor remote and execute script' ' ++ rm -f script-executed && ++ test_must_fail git clone \ ++ --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ ++ "file://$(pwd)/evil" clone2 2>err && ++ ! grep "fake-upload-pack running" err && ++ test_path_is_missing script-executed ++' ++ ++test_expect_failure 'fetch from file://... must not fetch from promisor remote and execute script' ' ++ rm -f script-executed && ++ test_must_fail git fetch \ ++ --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ ++ "file://$(pwd)/evil" 2>err && ++ ! grep "fake-upload-pack running" err && ++ test_path_is_missing script-executed ++' ++ ++test_expect_success 'pack-objects should fetch from promisor remote and execute script' ' ++ rm -f script-executed && ++ echo "HEAD" | test_must_fail git -C evil pack-objects --revs --stdout >/dev/null 2>err && ++ grep "fake-upload-pack running" err && ++ test_path_is_file script-executed ++' ++ ++test_done +-- +2.33.0 + diff --git a/backport-CVE-2024-32020-builtin-clone-refuse-local-clones-of-unsafe-reposito.patch b/backport-CVE-2024-32020-builtin-clone-refuse-local-clones-of-unsafe-reposito.patch new file mode 100644 index 0000000000000000000000000000000000000000..79abdf48b05e50c85a4fefb6b283c644a46da1c7 --- /dev/null +++ b/backport-CVE-2024-32020-builtin-clone-refuse-local-clones-of-unsafe-reposito.patch @@ -0,0 +1,109 @@ +From 1204e1a824c34071019fe106348eaa6d88f9528d Mon Sep 17 00:00:00 2001 +From: Patrick Steinhardt +Date: Mon, 15 Apr 2024 13:30:41 +0200 +Subject: [PATCH] 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 +Signed-off-by: Johannes Schindelin +--- + builtin/clone.c | 14 ++++++++++++++ + t/t0033-safe-directory.sh | 24 ++++++++++++++++++++++++ + 2 files changed, 38 insertions(+) + +diff --git a/builtin/clone.c b/builtin/clone.c +index 4b80fa087..9ec500d42 100644 +--- a/builtin/clone.c ++++ b/builtin/clone.c +@@ -321,6 +321,20 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, + 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); +diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh +index dc3496897..11c3e8f28 100755 +--- a/t/t0033-safe-directory.sh ++++ b/t/t0033-safe-directory.sh +@@ -80,4 +80,28 @@ test_expect_success 'safe.directory in included file' ' + 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 +-- +2.33.0 + diff --git a/backport-CVE-2024-32021-builtin-clone-abort-when-hardlinked-source-and-targe.patch b/backport-CVE-2024-32021-builtin-clone-abort-when-hardlinked-source-and-targe.patch new file mode 100644 index 0000000000000000000000000000000000000000..2afafc77e526234934533027185f38c1f8483242 --- /dev/null +++ b/backport-CVE-2024-32021-builtin-clone-abort-when-hardlinked-source-and-targe.patch @@ -0,0 +1,60 @@ +From d1bb66a546b4bb46005d17ba711caaad26f26c1e Mon Sep 17 00:00:00 2001 +From: Patrick Steinhardt +Date: Mon, 15 Apr 2024 13:30:31 +0200 +Subject: [PATCH] builtin/clone: abort when hardlinked source and target file + differ + +When performing local clones with hardlinks we refuse to copy source +files which are symlinks as a mitigation for CVE-2022-39253. This check +can be raced by an adversary though by changing the file to a symlink +after we have checked it. + +Fix the issue by checking whether the hardlinked destination file +matches the source file and abort in case it doesn't. + +This addresses CVE-2024-32021. + +Reported-by: Apple Product Security +Suggested-by: Linus Torvalds +Signed-off-by: Patrick Steinhardt +Signed-off-by: Johannes Schindelin +--- + builtin/clone.c | 21 ++++++++++++++++++++- + 1 file changed, 20 insertions(+), 1 deletion(-) + +diff --git a/builtin/clone.c b/builtin/clone.c +index 073e6323d..4b80fa087 100644 +--- a/builtin/clone.c ++++ b/builtin/clone.c +@@ -357,8 +357,27 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, + if (unlink(dest->buf) && errno != ENOENT) + die_errno(_("failed to unlink '%s'"), dest->buf); + if (!option_no_hardlinks) { +- if (!link(src->buf, dest->buf)) ++ if (!link(src->buf, dest->buf)) { ++ struct stat st; ++ ++ /* ++ * Sanity-check whether the created hardlink ++ * actually links to the expected file now. This ++ * catches time-of-check-time-of-use bugs in ++ * case the source file was meanwhile swapped. ++ */ ++ if (lstat(dest->buf, &st)) ++ die(_("hardlink cannot be checked at '%s'"), dest->buf); ++ if (st.st_mode != iter->st.st_mode || ++ st.st_ino != iter->st.st_ino || ++ st.st_dev != iter->st.st_dev || ++ st.st_size != iter->st.st_size || ++ st.st_uid != iter->st.st_uid || ++ st.st_gid != iter->st.st_gid) ++ die(_("hardlink different from source at '%s'"), dest->buf); ++ + continue; ++ } + if (option_local > 0) + die_errno(_("failed to create link '%s'"), dest->buf); + option_no_hardlinks = 1; +-- +2.33.0 + diff --git a/backport-CVE-2024-32021-builtin-clone-stop-resolving-symlinks-when-copying-f.patch b/backport-CVE-2024-32021-builtin-clone-stop-resolving-symlinks-when-copying-f.patch new file mode 100644 index 0000000000000000000000000000000000000000..fb0fc5e6c3b52a13a0ab63cfdacea911085a9486 --- /dev/null +++ b/backport-CVE-2024-32021-builtin-clone-stop-resolving-symlinks-when-copying-f.patch @@ -0,0 +1,84 @@ +From 150e6b0aedf57d224c3c49038c306477fa159886 Mon Sep 17 00:00:00 2001 +From: Patrick Steinhardt +Date: Mon, 15 Apr 2024 13:30:26 +0200 +Subject: [PATCH] builtin/clone: stop resolving symlinks when copying files + +When a user performs a local clone without `--no-local`, then we end up +copying the source repository into the target repository directly. To +optimize this even further, we try to hardlink files into place instead +of copying data over, which helps both disk usage and speed. + +There is an important edge case in this context though, namely when we +try to hardlink symlinks from the source repository into the target +repository. Depending on both platform and filesystem the resulting +behaviour here can be different: + + - On macOS and NetBSD, calling link(3P) with a symlink target creates + a hardlink to the file pointed to by the symlink. + + - On Linux, calling link(3P) instead creates a hardlink to the symlink + itself. + +To unify this behaviour, 36596fd2df (clone: better handle symlinked +files at .git/objects/, 2019-07-10) introduced logic to resolve symlinks +before we try to link(3P) files. Consequently, the new behaviour was to +always create a hard link to the target of the symlink on all platforms. + +Eventually though, we figured out that following symlinks like this can +cause havoc when performing a local clone of a malicious repository, +which resulted in CVE-2022-39253. This issue was fixed via 6f054f9fb3 +(builtin/clone.c: disallow `--local` clones with symlinks, 2022-07-28), +by refusing symlinks in the source repository. + +But even though we now shouldn't ever link symlinks anymore, the code +that resolves symlinks still exists. In the best case the code does not +end up doing anything because there are no symlinks anymore. In the +worst case though this can be abused by an adversary that rewrites the +source file after it has been checked not to be a symlink such that it +actually is a symlink when we call link(3P). Thus, it is still possible +to recreate CVE-2022-39253 due to this time-of-check-time-of-use bug. + +Remove the call to `realpath()`. This doesn't yet address the actual +vulnerability, which will be handled in a subsequent commit. + +Reported-by: Apple Product Security +Signed-off-by: Patrick Steinhardt +Signed-off-by: Johannes Schindelin +--- + builtin/clone.c | 6 +----- + 1 file changed, 1 insertion(+), 5 deletions(-) + +diff --git a/builtin/clone.c b/builtin/clone.c +index 3c2ae31a5..073e6323d 100644 +--- a/builtin/clone.c ++++ b/builtin/clone.c +@@ -320,7 +320,6 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, + int src_len, dest_len; + struct dir_iterator *iter; + int iter_status; +- struct strbuf realpath = STRBUF_INIT; + + mkdir_if_missing(dest->buf, 0777); + +@@ -358,8 +357,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, + if (unlink(dest->buf) && errno != ENOENT) + die_errno(_("failed to unlink '%s'"), dest->buf); + if (!option_no_hardlinks) { +- strbuf_realpath(&realpath, src->buf, 1); +- if (!link(realpath.buf, dest->buf)) ++ if (!link(src->buf, dest->buf)) + continue; + if (option_local > 0) + die_errno(_("failed to create link '%s'"), dest->buf); +@@ -373,8 +371,6 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, + strbuf_setlen(src, src_len); + die(_("failed to iterate over '%s'"), src->buf); + } +- +- strbuf_release(&realpath); + } + + static void clone_local(const char *src_repo, const char *dest_repo) +-- +2.33.0 + diff --git a/backport-CVE-2024-32465-upload-pack-disable-lazy-fetching-by-default.patch b/backport-CVE-2024-32465-upload-pack-disable-lazy-fetching-by-default.patch new file mode 100644 index 0000000000000000000000000000000000000000..a5b8b6a9aa77ce5be12c3d9fd94c70ff7997762d --- /dev/null +++ b/backport-CVE-2024-32465-upload-pack-disable-lazy-fetching-by-default.patch @@ -0,0 +1,201 @@ +From 7b70e9efb18c2cc3f219af399bd384c5801ba1d7 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Tue, 16 Apr 2024 04:35:33 -0400 +Subject: [PATCH] upload-pack: disable lazy-fetching by default + +The upload-pack command tries to avoid trusting the repository in which +it's run (e.g., by not running any hooks and not using any config that +contains arbitrary commands). But if the server side of a fetch or a +clone is a partial clone, then either upload-pack or its child +pack-objects may run a lazy "git fetch" under the hood. And it is very +easy to convince fetch to run arbitrary commands. + +The "server" side can be a local repository owned by someone else, who +would be able to configure commands that are run during a clone with the +current user's permissions. This issue has been designated +CVE-2024-32004. + +The fix in this commit's parent helps in this scenario, as well as in +related scenarios using SSH to clone, where the untrusted .git directory +is owned by a different user id. But if you received one as a zip file, +on a USB stick, etc, it may be owned by your user but still untrusted. + +This has been designated CVE-2024-32465. + +To mitigate the issue more completely, let's disable lazy fetching +entirely during `upload-pack`. While fetching from a partial repository +should be relatively rare, it is certainly not an unreasonable workflow. +And thus we need to provide an escape hatch. + +This commit works by respecting a GIT_NO_LAZY_FETCH environment variable +(to skip the lazy-fetch), and setting it in upload-pack, but only when +the user has not already done so (which gives us the escape hatch). + +The name of the variable is specifically chosen to match what has +already been added in 'master' via e6d5479e7a (git: extend +--no-lazy-fetch to work across subprocesses, 2024-02-27). Since we're +building this fix as a backport for older versions, we could cherry-pick +that patch and its earlier steps. However, we don't really need the +niceties (like a "--no-lazy-fetch" option) that it offers. By using the +same name, everything should just work when the two are eventually +merged, but here are a few notes: + + - the blocking of the fetch in e6d5479e7a is incomplete! It sets + fetch_if_missing to 0 when we setup the repository variable, but + that isn't enough. pack-objects in particular will call + prefetch_to_pack() even if that variable is 0. This patch by + contrast checks the environment variable at the lowest level before + we call the lazy fetch, where we can be sure to catch all code + paths. + + Possibly the setting of fetch_if_missing from e6d5479e7a can be + reverted, but it may be useful to have. For example, some code may + want to use that flag to change behavior before it gets to the point + of trying to start the fetch. At any rate, that's all outside the + scope of this patch. + + - there's documentation for GIT_NO_LAZY_FETCH in e6d5479e7a. We can + live without that here, because for the most part the user shouldn't + need to set it themselves. The exception is if they do want to + override upload-pack's default, and that requires a separate + documentation section (which is added here) + + - it would be nice to use the NO_LAZY_FETCH_ENVIRONMENT macro added by + e6d5479e7a, but those definitions have moved from cache.h to + environment.h between 2.39.3 and master. I just used the raw string + literals, and we can replace them with the macro once this topic is + merged to master. + +At least with respect to CVE-2024-32004, this does render this commit's +parent commit somewhat redundant. However, it is worth retaining that +commit as defense in depth, and because it may help other issues (e.g., +symlink/hardlink TOCTOU races, where zip files are not really an +interesting attack vector). + +The tests in t0411 still pass, but now we have _two_ mechanisms ensuring +that the evil command is not run. Let's beef up the existing ones to +check that they failed for the expected reason, that we refused to run +upload-pack at all with an alternate user id. And add two new ones for +the same-user case that both the restriction and its escape hatch. + +Signed-off-by: Jeff King +Signed-off-by: Johannes Schindelin +--- + Documentation/git-upload-pack.txt | 16 ++++++++++++++++ + builtin/upload-pack.c | 2 ++ + promisor-remote.c | 10 ++++++++++ + t/t0411-clone-from-partial.sh | 18 ++++++++++++++++++ + 4 files changed, 46 insertions(+) + +diff --git a/Documentation/git-upload-pack.txt b/Documentation/git-upload-pack.txt +index b656b4756..fc4c62d7b 100644 +--- a/Documentation/git-upload-pack.txt ++++ b/Documentation/git-upload-pack.txt +@@ -55,6 +55,22 @@ ENVIRONMENT + admins may need to configure some transports to allow this + variable to be passed. See the discussion in linkgit:git[1]. + ++`GIT_NO_LAZY_FETCH`:: ++ When cloning or fetching from a partial repository (i.e., one ++ itself cloned with `--filter`), the server-side `upload-pack` ++ may need to fetch extra objects from its upstream in order to ++ complete the request. By default, `upload-pack` will refuse to ++ perform such a lazy fetch, because `git fetch` may run arbitrary ++ commands specified in configuration and hooks of the source ++ repository (and `upload-pack` tries to be safe to run even in ++ untrusted `.git` directories). +++ ++This is implemented by having `upload-pack` internally set the ++`GIT_NO_LAZY_FETCH` variable to `1`. If you want to override it ++(because you are fetching from a partial clone, and you are sure ++you trust it), you can explicitly set `GIT_NO_LAZY_FETCH` to ++`0`. ++ + SEE ALSO + -------- + linkgit:gitnamespaces[7] +diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c +index 25b69da2b..f446ff04f 100644 +--- a/builtin/upload-pack.c ++++ b/builtin/upload-pack.c +@@ -35,6 +35,8 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix) + + packet_trace_identity("upload-pack"); + disable_replace_refs(); ++ /* TODO: This should use NO_LAZY_FETCH_ENVIRONMENT */ ++ xsetenv("GIT_NO_LAZY_FETCH", "1", 0); + + argc = parse_options(argc, argv, prefix, options, upload_pack_usage, 0); + +diff --git a/promisor-remote.c b/promisor-remote.c +index faa761294..550a38f75 100644 +--- a/promisor-remote.c ++++ b/promisor-remote.c +@@ -20,6 +20,16 @@ static int fetch_objects(struct repository *repo, + int i; + FILE *child_in; + ++ /* TODO: This should use NO_LAZY_FETCH_ENVIRONMENT */ ++ if (git_env_bool("GIT_NO_LAZY_FETCH", 0)) { ++ static int warning_shown; ++ if (!warning_shown) { ++ warning_shown = 1; ++ warning(_("lazy fetching disabled; some objects may not be available")); ++ } ++ return -1; ++ } ++ + child.git_cmd = 1; + child.in = -1; + if (repo != the_repository) +diff --git a/t/t0411-clone-from-partial.sh b/t/t0411-clone-from-partial.sh +index eb3360dbc..b3d6ddc4b 100755 +--- a/t/t0411-clone-from-partial.sh ++++ b/t/t0411-clone-from-partial.sh +@@ -28,6 +28,7 @@ test_expect_success 'local clone must not fetch from promisor remote and execute + test_must_fail git clone \ + --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ + evil clone1 2>err && ++ grep "detected dubious ownership" err && + ! grep "fake-upload-pack running" err && + test_path_is_missing script-executed + ' +@@ -37,6 +38,7 @@ test_expect_success 'clone from file://... must not fetch from promisor remote a + test_must_fail git clone \ + --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ + "file://$(pwd)/evil" clone2 2>err && ++ grep "detected dubious ownership" err && + ! grep "fake-upload-pack running" err && + test_path_is_missing script-executed + ' +@@ -46,6 +48,7 @@ test_expect_success 'fetch from file://... must not fetch from promisor remote a + test_must_fail git fetch \ + --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ + "file://$(pwd)/evil" 2>err && ++ grep "detected dubious ownership" err && + ! grep "fake-upload-pack running" err && + test_path_is_missing script-executed + ' +@@ -57,4 +60,19 @@ test_expect_success 'pack-objects should fetch from promisor remote and execute + test_path_is_file script-executed + ' + ++test_expect_success 'clone from promisor remote does not lazy-fetch by default' ' ++ rm -f script-executed && ++ test_must_fail git clone evil no-lazy 2>err && ++ grep "lazy fetching disabled" err && ++ test_path_is_missing script-executed ++' ++ ++test_expect_success 'promisor lazy-fetching can be re-enabled' ' ++ rm -f script-executed && ++ test_must_fail env GIT_NO_LAZY_FETCH=0 \ ++ git clone evil lazy-ok 2>err && ++ grep "fake-upload-pack running" err && ++ test_path_is_file script-executed ++' ++ + test_done +-- +2.33.0 + diff --git a/git.spec b/git.spec index 6574bdfcfa26bcef1deb07dcdff6c20e4616d718..111fa1a22e024f6e279b2a3f69a434f835801a8d 100644 --- a/git.spec +++ b/git.spec @@ -1,7 +1,7 @@ %global gitexecdir %{_libexecdir}/git-core Name: git Version: 2.44.0 -Release: 2 +Release: 3 Summary: A popular and widely used Version Control System License: GPLv2+ or LGPLv2.1 URL: https://git-scm.com/ @@ -13,6 +13,12 @@ Source101: git@.service.in Source102: git.socket Patch0: backport-CVE-2024-32002-submodules-submodule-paths-m.patch +Patch1: backport-CVE-2024-32021-builtin-clone-stop-resolving-symlinks-when-copying-f.patch +Patch2: backport-CVE-2024-32021-builtin-clone-abort-when-hardlinked-source-and-targe.patch +Patch3: backport-CVE-2024-32004-t0411-add-tests-for-cloning-from-partial-repo.patch +Patch4: backport-CVE-2024-32004-fetch-clone-detect-dubious-ownership-of-local-reposi.patch +Patch5: backport-CVE-2024-32020-builtin-clone-refuse-local-clones-of-unsafe-reposito.patch +Patch6: backport-CVE-2024-32465-upload-pack-disable-lazy-fetching-by-default.patch BuildRequires: gcc gettext BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre2-devel desktop-file-utils @@ -297,6 +303,12 @@ make %{?_smp_mflags} test %{_mandir}/man7/git*.7.* %changelog +* Thu May 16 2024 fuanan - 2.44.0-3 +- Type:CVE +- ID:CVE-2024-32021 CVE-2024-32004 CVE-2024-32020 CVE-2024-32465 +- SUG:NA +- DESC:Fix CVE-2024-32021 CVE-2024-32004 CVE-2024-32020 CVE-2024-32465 + * Thu May 16 2024 qiaojijun - 2.44.0-2 - Type:CVE - ID:CVE-2024-32002