Merge branch 'ps/apply-beyond-symlink' into maint-2.30

Fix a vulnerability (CVE-2023-23946) that allows crafted input to trick
`git apply` into writing files outside of the working tree.

* ps/apply-beyond-symlink:
  dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This commit is contained in:
Junio C Hamano 2023-02-03 14:57:27 -08:00 committed by Johannes Schindelin
commit a3033a68ac
2 changed files with 108 additions and 0 deletions

27
apply.c
View File

@ -4400,6 +4400,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

@ -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