1
0
Fork 0
mirror of https://github.com/git/git.git synced 2024-05-28 05:46:38 +02:00

Sync with 2.32.6

* maint-2.32:
  Git 2.32.6
  Git 2.31.7
  Git 2.30.8
  apply: fix writing behind newly created symbolic links
  dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
  clone: delay picking a transport until after get_repo_path()
  t5619: demonstrate clone_local() with ambiguous transport
This commit is contained in:
Johannes Schindelin 2023-02-06 09:25:56 +01:00
commit 87248c5933
11 changed files with 302 additions and 9 deletions

View File

@ -0,0 +1,52 @@
Git v2.30.8 Release Notes
=========================
This release addresses the security issues CVE-2023-22490 and
CVE-2023-23946.
Fixes since v2.30.7
-------------------
* CVE-2023-22490:
Using a specially-crafted repository, Git can be tricked into using
its local clone optimization even when using a non-local transport.
Though Git will abort local clones whose source $GIT_DIR/objects
directory contains symbolic links (c.f., CVE-2022-39253), the objects
directory itself may still be a symbolic link.
These two may be combined to include arbitrary files based on known
paths on the victim's filesystem within the malicious repository's
working copy, allowing for data exfiltration in a similar manner as
CVE-2022-39253.
* CVE-2023-23946:
By feeding a crafted input to "git apply", a path outside the
working tree can be overwritten as the user who is running "git
apply".
* A mismatched type in `attr.c::read_attr_from_index()` which could
cause Git to errantly reject attributes on Windows and 32-bit Linux
has been corrected.
Credit for finding CVE-2023-22490 goes to yvvdwf, and the fix was
developed by Taylor Blau, with additional help from others on the
Git security mailing list.
Credit for finding CVE-2023-23946 goes to Joern Schneeweisz, and the
fix was developed by Patrick Steinhardt.
Johannes Schindelin (1):
attr: adjust a mismatched data type
Patrick Steinhardt (1):
apply: fix writing behind newly created symbolic links
Taylor Blau (3):
t5619: demonstrate clone_local() with ambiguous transport
clone: delay picking a transport until after get_repo_path()
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS

View File

@ -0,0 +1,6 @@
Git v2.31.7 Release Notes
=========================
This release merges up the fixes that appear in v2.30.8 to
address the security issues CVE-2023-22490 and CVE-2023-23946;
see the release notes for that version for details.

View File

@ -0,0 +1,6 @@
Git v2.32.6 Release Notes
=========================
This release merges up the fixes that appear in v2.30.8 and v2.31.7
to address the security issues CVE-2023-22490 and CVE-2023-23946;
see the release notes for these versions for details.

27
apply.c
View File

@ -4424,6 +4424,33 @@ static int create_one_file(struct apply_state *state,
if (state->cached)
return 0;
/*
* We already try to detect whether files are beyond a symlink in our
* up-front checks. But in the case where symlinks are created by any
* of the intermediate hunks it can happen that our up-front checks
* didn't yet see the symlink, but at the point of arriving here there
* in fact is one. We thus repeat the check for symlinks here.
*
* Note that this does not make the up-front check obsolete as the
* failure mode is different:
*
* - The up-front checks cause us to abort before we have written
* anything into the working directory. So when we exit this way the
* working directory remains clean.
*
* - The checks here happen in the middle of the action where we have
* already started to apply the patch. The end result will be a dirty
* working directory.
*
* Ideally, we should update the up-front checks to catch what would
* happen when we apply the patch before we damage the working tree.
* We have all the information necessary to do so. But for now, as a
* part of embargoed security work, having this check would serve as a
* reasonable first step.
*/
if (path_is_beyond_symlink(state, path))
return error(_("affected file '%s' is beyond a symbolic link"), path);
res = try_create_file(state, path, mode, buf, size);
if (res < 0)
return -1;

View File

@ -1220,10 +1220,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
refspec_appendf(&remote->fetch, "+%s*:%s*", src_ref_prefix,
branch_top.buf);
transport = transport_get(remote, remote->url[0]);
transport_set_verbosity(transport, option_verbosity, option_progress);
transport->family = family;
path = get_repo_path(remote->url[0], &is_bundle);
is_local = option_local != 0 && path && !is_bundle;
if (is_local) {
@ -1245,6 +1241,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
}
if (option_local > 0 && !is_local)
warning(_("--local is ignored"));
transport = transport_get(remote, path ? path : remote->url[0]);
transport_set_verbosity(transport, option_verbosity, option_progress);
transport->family = family;
transport->cloning = 1;
transport_set_option(transport, TRANS_OPT_KEEP, "yes");

View File

@ -203,7 +203,7 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)
{
struct dir_iterator_int *iter = xcalloc(1, sizeof(*iter));
struct dir_iterator *dir_iterator = &iter->base;
int saved_errno;
int saved_errno, err;
strbuf_init(&iter->base.path, PATH_MAX);
strbuf_addstr(&iter->base.path, path);
@ -213,10 +213,15 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)
iter->flags = flags;
/*
* Note: stat already checks for NULL or empty strings and
* inexistent paths.
* Note: stat/lstat already checks for NULL or empty strings and
* nonexistent paths.
*/
if (stat(iter->base.path.buf, &iter->base.st) < 0) {
if (iter->flags & DIR_ITERATOR_FOLLOW_SYMLINKS)
err = stat(iter->base.path.buf, &iter->base.st);
else
err = lstat(iter->base.path.buf, &iter->base.st);
if (err < 0) {
saved_errno = errno;
goto error_out;
}

View File

@ -61,6 +61,11 @@
* not the symlinks themselves, which is the default behavior. Broken
* symlinks are ignored.
*
* Note: setting DIR_ITERATOR_FOLLOW_SYMLINKS affects resolving the
* starting path as well (e.g., attempting to iterate starting at a
* symbolic link pointing to a directory without FOLLOW_SYMLINKS will
* result in an error).
*
* Warning: circular symlinks are also followed when
* DIR_ITERATOR_FOLLOW_SYMLINKS is set. The iteration may end up with
* an ELOOP if they happen and DIR_ITERATOR_PEDANTIC is set.

View File

@ -109,7 +109,9 @@ test_expect_success SYMLINKS 'setup dirs with symlinks' '
mkdir -p dir5/a/c &&
ln -s ../c dir5/a/b/d &&
ln -s ../ dir5/a/b/e &&
ln -s ../../ dir5/a/b/f
ln -s ../../ dir5/a/b/f &&
ln -s dir4 dir6
'
test_expect_success SYMLINKS 'dir-iterator should not follow symlinks by default' '
@ -145,4 +147,27 @@ test_expect_success SYMLINKS 'dir-iterator should follow symlinks w/ follow flag
test_cmp expected-follow-sorted-output actual-follow-sorted-output
'
test_expect_success SYMLINKS 'dir-iterator does not resolve top-level symlinks' '
test_must_fail test-tool dir-iterator ./dir6 >out &&
grep "ENOTDIR" out
'
test_expect_success SYMLINKS 'dir-iterator resolves top-level symlinks w/ follow flag' '
cat >expected-follow-sorted-output <<-EOF &&
[d] (a) [a] ./dir6/a
[d] (a/f) [f] ./dir6/a/f
[d] (a/f/c) [c] ./dir6/a/f/c
[d] (b) [b] ./dir6/b
[d] (b/c) [c] ./dir6/b/c
[f] (a/d) [d] ./dir6/a/d
[f] (a/e) [e] ./dir6/a/e
EOF
test-tool dir-iterator --follow-symlinks ./dir6 >out &&
sort out >actual-follow-sorted-output &&
test_cmp expected-follow-sorted-output actual-follow-sorted-output
'
test_done

View File

@ -44,4 +44,85 @@ test_expect_success 'apply --index symlink patch' '
'
test_expect_success 'symlink setup' '
ln -s .git symlink &&
git add symlink &&
git commit -m "add symlink"
'
test_expect_success SYMLINKS 'symlink escape when creating new files' '
test_when_finished "git reset --hard && git clean -dfx" &&
cat >patch <<-EOF &&
diff --git a/symlink b/renamed-symlink
similarity index 100%
rename from symlink
rename to renamed-symlink
--
diff --git /dev/null b/renamed-symlink/create-me
new file mode 100644
index 0000000..039727e
--- /dev/null
+++ b/renamed-symlink/create-me
@@ -0,0 +1,1 @@
+busted
EOF
test_must_fail git apply patch 2>stderr &&
cat >expected_stderr <<-EOF &&
error: affected file ${SQ}renamed-symlink/create-me${SQ} is beyond a symbolic link
EOF
test_cmp expected_stderr stderr &&
! test_path_exists .git/create-me
'
test_expect_success SYMLINKS 'symlink escape when modifying file' '
test_when_finished "git reset --hard && git clean -dfx" &&
touch .git/modify-me &&
cat >patch <<-EOF &&
diff --git a/symlink b/renamed-symlink
similarity index 100%
rename from symlink
rename to renamed-symlink
--
diff --git a/renamed-symlink/modify-me b/renamed-symlink/modify-me
index 1111111..2222222 100644
--- a/renamed-symlink/modify-me
+++ b/renamed-symlink/modify-me
@@ -0,0 +1,1 @@
+busted
EOF
test_must_fail git apply patch 2>stderr &&
cat >expected_stderr <<-EOF &&
error: renamed-symlink/modify-me: No such file or directory
EOF
test_cmp expected_stderr stderr &&
test_must_be_empty .git/modify-me
'
test_expect_success SYMLINKS 'symlink escape when deleting file' '
test_when_finished "git reset --hard && git clean -dfx && rm .git/delete-me" &&
touch .git/delete-me &&
cat >patch <<-EOF &&
diff --git a/symlink b/renamed-symlink
similarity index 100%
rename from symlink
rename to renamed-symlink
--
diff --git a/renamed-symlink/delete-me b/renamed-symlink/delete-me
deleted file mode 100644
index 1111111..0000000 100644
EOF
test_must_fail git apply patch 2>stderr &&
cat >expected_stderr <<-EOF &&
error: renamed-symlink/delete-me: No such file or directory
EOF
test_cmp expected_stderr stderr &&
test_path_is_file .git/delete-me
'
test_done

View File

@ -344,4 +344,20 @@ test_expect_success SYMLINKS 'clone repo with symlinked or unknown files at obje
test_must_be_empty T--shared.objects-symlinks.raw
'
test_expect_success SYMLINKS 'clone repo with symlinked objects directory' '
test_when_finished "rm -fr sensitive malicious" &&
mkdir -p sensitive &&
echo "secret" >sensitive/file &&
git init malicious &&
rm -fr malicious/.git/objects &&
ln -s "$(pwd)/sensitive" ./malicious/.git/objects &&
test_must_fail git clone --local malicious clone 2>err &&
test_path_is_missing clone &&
grep "failed to start iterator over" err
'
test_done

View File

@ -0,0 +1,70 @@
#!/bin/sh
test_description='test local clone with ambiguous transport'
. ./test-lib.sh
. "$TEST_DIRECTORY/lib-httpd.sh"
if ! test_have_prereq SYMLINKS
then
skip_all='skipping test, symlink support unavailable'
test_done
fi
start_httpd
REPO="$HTTPD_DOCUMENT_ROOT_PATH/sub.git"
URI="$HTTPD_URL/dumb/sub.git"
test_expect_success 'setup' '
mkdir -p sensitive &&
echo "secret" >sensitive/secret &&
git init --bare "$REPO" &&
test_commit_bulk -C "$REPO" --ref=main 1 &&
git -C "$REPO" update-ref HEAD main &&
git -C "$REPO" update-server-info &&
git init malicious &&
(
cd malicious &&
git submodule add "$URI" &&
mkdir -p repo/refs &&
touch repo/refs/.gitkeep &&
printf "ref: refs/heads/a" >repo/HEAD &&
ln -s "$(cd .. && pwd)/sensitive" repo/objects &&
mkdir -p "$HTTPD_URL/dumb" &&
ln -s "../../../.git/modules/sub/../../../repo/" "$URI" &&
git add . &&
git commit -m "initial commit"
) &&
# Delete all of the references in our malicious submodule to
# avoid the client attempting to checkout any objects (which
# will be missing, and thus will cause the clone to fail before
# we can trigger the exploit).
git -C "$REPO" for-each-ref --format="delete %(refname)" >in &&
git -C "$REPO" update-ref --stdin <in &&
git -C "$REPO" update-server-info
'
test_expect_success 'ambiguous transport does not lead to arbitrary file-inclusion' '
git clone malicious clone &&
test_must_fail git -C clone submodule update --init 2>err &&
test_path_is_missing clone/.git/modules/sub/objects/secret &&
# We would actually expect "transport .file. not allowed" here,
# but due to quirks of the URL detection in Git, we mis-parse
# the absolute path as a bogus URL and die before that step.
#
# This works for now, and if we ever fix the URL detection, it
# is OK to change this to detect the transport error.
grep "protocol .* is not supported" err
'
test_done