mirror of
https://github.com/git/git
synced 2024-08-25 02:35:46 +00:00
Sync with 2.31.7
* maint-2.31: 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:
commit
aeb93d7da2
52
Documentation/RelNotes/2.30.8.txt
Normal file
52
Documentation/RelNotes/2.30.8.txt
Normal 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
|
||||||
|
|
6
Documentation/RelNotes/2.31.7.txt
Normal file
6
Documentation/RelNotes/2.31.7.txt
Normal 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.
|
27
apply.c
27
apply.c
|
@ -4402,6 +4402,33 @@ static int create_one_file(struct apply_state *state,
|
||||||
if (state->cached)
|
if (state->cached)
|
||||||
return 0;
|
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);
|
res = try_create_file(state, path, mode, buf, size);
|
||||||
if (res < 0)
|
if (res < 0)
|
||||||
return -1;
|
return -1;
|
||||||
|
|
|
@ -1220,10 +1220,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
|
||||||
refspec_appendf(&remote->fetch, "+%s*:%s*", src_ref_prefix,
|
refspec_appendf(&remote->fetch, "+%s*:%s*", src_ref_prefix,
|
||||||
branch_top.buf);
|
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);
|
path = get_repo_path(remote->url[0], &is_bundle);
|
||||||
is_local = option_local != 0 && path && !is_bundle;
|
is_local = option_local != 0 && path && !is_bundle;
|
||||||
if (is_local) {
|
if (is_local) {
|
||||||
|
@ -1245,6 +1241,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
|
||||||
}
|
}
|
||||||
if (option_local > 0 && !is_local)
|
if (option_local > 0 && !is_local)
|
||||||
warning(_("--local is ignored"));
|
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->cloning = 1;
|
||||||
|
|
||||||
transport_set_option(transport, TRANS_OPT_KEEP, "yes");
|
transport_set_option(transport, TRANS_OPT_KEEP, "yes");
|
||||||
|
|
|
@ -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_int *iter = xcalloc(1, sizeof(*iter));
|
||||||
struct dir_iterator *dir_iterator = &iter->base;
|
struct dir_iterator *dir_iterator = &iter->base;
|
||||||
int saved_errno;
|
int saved_errno, err;
|
||||||
|
|
||||||
strbuf_init(&iter->base.path, PATH_MAX);
|
strbuf_init(&iter->base.path, PATH_MAX);
|
||||||
strbuf_addstr(&iter->base.path, path);
|
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;
|
iter->flags = flags;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Note: stat already checks for NULL or empty strings and
|
* Note: stat/lstat already checks for NULL or empty strings and
|
||||||
* inexistent paths.
|
* 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;
|
saved_errno = errno;
|
||||||
goto error_out;
|
goto error_out;
|
||||||
}
|
}
|
||||||
|
|
|
@ -61,6 +61,11 @@
|
||||||
* not the symlinks themselves, which is the default behavior. Broken
|
* not the symlinks themselves, which is the default behavior. Broken
|
||||||
* symlinks are ignored.
|
* 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
|
* Warning: circular symlinks are also followed when
|
||||||
* DIR_ITERATOR_FOLLOW_SYMLINKS is set. The iteration may end up with
|
* DIR_ITERATOR_FOLLOW_SYMLINKS is set. The iteration may end up with
|
||||||
* an ELOOP if they happen and DIR_ITERATOR_PEDANTIC is set.
|
* an ELOOP if they happen and DIR_ITERATOR_PEDANTIC is set.
|
||||||
|
|
|
@ -109,7 +109,9 @@ test_expect_success SYMLINKS 'setup dirs with symlinks' '
|
||||||
mkdir -p dir5/a/c &&
|
mkdir -p dir5/a/c &&
|
||||||
ln -s ../c dir5/a/b/d &&
|
ln -s ../c dir5/a/b/d &&
|
||||||
ln -s ../ dir5/a/b/e &&
|
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' '
|
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_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
|
test_done
|
||||||
|
|
|
@ -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
|
test_done
|
||||||
|
|
|
@ -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_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
|
test_done
|
||||||
|
|
70
t/t5619-clone-local-ambiguous-transport.sh
Executable file
70
t/t5619-clone-local-ambiguous-transport.sh
Executable 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
|
Loading…
Reference in a new issue