aboutsummaryrefslogtreecommitdiffstats
path: root/apply.c
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2023-02-03 14:57:27 -0800
committerJohannes Schindelin <johannes.schindelin@gmx.de>2023-02-06 09:12:16 +0100
commita3033a68ac3886d44ee378784ae242f25afc9970 (patch)
tree72737d582c2371a1b440bb630502398005eddb40 /apply.c
parentMerge branch 'tb/clone-local-symlinks' into maint-2.30 (diff)
parentapply: fix writing behind newly created symbolic links (diff)
downloadgit-a3033a68ac3886d44ee378784ae242f25afc9970.tar.gz
git-a3033a68ac3886d44ee378784ae242f25afc9970.zip
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>
Diffstat (limited to '')
-rw-r--r--apply.c27
1 files changed, 27 insertions, 0 deletions
diff --git a/apply.c b/apply.c
index 668b16e989..d80382c940 100644
--- a/apply.c
+++ b/apply.c
@@ -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;