git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 1/4] apply: reject input that touches outside the working area

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 10, 2015, 22:36 UTC
Message-ID
<1423607771-27157-2-git-send-email-gitster@pobox.com>
In-Reply-To
<1423607771-27157-1-git-send-email-gitster@pobox.com>

By default, a patch that affects outside the working area (either a Git controlled working tree, or the current working directory when "git apply" is used as a replacement of GNU patch) is rejected as a mistake (or a mischief). Git itself does not create such a patch, unless the user bends over backwards and specifies a non-standard prefix to "git diff" and friends.

When `git apply` is used as a "better GNU patch", the user can pass the `--unsafe-paths` option to override this safety check. This option has no effect when `--index` or `--cached` is in use.

The new test was stolen from Jeff King with slight enhancements. Note that a few new tests for touching outside the working area by following a symbolic link are still expected to fail at this step, but will be fixed in later steps.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-apply.txt |  12 +++-
 builtin/apply.c             |  26 ++++++++
 t/t4139-apply-escape.sh     | 141 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+), 1 deletion(-)
 create mode 100755 t/t4139-apply-escape.sh
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index f605327..9489664 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 	  [--ignore-space-change | --ignore-whitespace ]
 	  [--whitespace=(nowarn|warn|fix|error|error-all)]
 	  [--exclude=<path>] [--include=<path>] [--directory=<root>]
-	  [--verbose] [<patch>...]
+	  [--verbose] [--unsafe-paths] [<patch>...]
 
 DESCRIPTION
 -----------
@@ -229,6 +229,16 @@ For example, a patch that talks about updating `a/git-gui.sh` to `b/git-gui.sh`
 can be applied to the file in the working tree `modules/git-gui/git-gui.sh` by
 running `git apply --directory=modules/git-gui`.
 
+--unsafe-paths::
+	By default, a patch that affects outside the working area
+	(either a Git controlled working tree, or the current working
+	directory when "git apply" is used as a replacement of GNU
+	patch) is rejected as a mistake (or a mischief).
++
+When `git apply` is used as a "better GNU patch", the user can pass
+the `--unsafe-paths` option to override this safety check.  This option
+has no effect when `--index` or `--cached` is in use.
+
 Configuration
 -------------
 
diff --git a/builtin/apply.c b/builtin/apply.c
index ef32e4f..8561236 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -50,6 +50,7 @@ static int apply_verbosely;
 static int allow_overlap;
 static int no_add;
 static int threeway;
+static int unsafe_paths;
 static const char *fake_ancestor;
 static int line_termination = '\n';
 static unsigned int p_context = UINT_MAX;
@@ -3483,6 +3484,23 @@ static int check_to_create(const char *new_name, int ok_if_exists)
 	return 0;
 }
 
+static void die_on_unsafe_path(struct patch *patch)
+{
+	const char *old_name = NULL;
+	const char *new_name = NULL;
+	if (patch->is_delete)
+		old_name = patch->old_name;
+	else if (!patch->is_new && !patch->is_copy)
+		old_name = patch->old_name;
+	if (!patch->is_delete)
+		new_name = patch->new_name;
+
+	if (old_name && !verify_path(old_name))
+		die(_("invalid path '%s'"), old_name);
+	if (new_name && !verify_path(new_name))
+		die(_("invalid path '%s'"), new_name);
+}
+
 /*
  * Check and apply the patch in-core; leave the result in patch->result
  * for the caller to write it out to the final destination.
@@ -3570,6 +3588,9 @@ static int check_patch(struct patch *patch)
 		}
 	}
 
+	if (!unsafe_paths)
+		die_on_unsafe_path(patch);
+
 	if (apply_data(patch, &st, ce) < 0)
 		return error(_("%s: patch does not apply"), name);
 	patch->rejected = 0;
@@ -4379,6 +4400,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
 			N_("make sure the patch is applicable to the current index")),
 		OPT_BOOL(0, "cached", &cached,
 			N_("apply a patch without touching the working tree")),
+		OPT_BOOL(0, "unsafe-paths", &unsafe_paths,
+			N_("accept a patch that touches outside the working area")),
 		OPT_BOOL(0, "apply", &force_apply,
 			N_("also apply the patch (use with --stat/--summary/--check)")),
 		OPT_BOOL('3', "3way", &threeway,
@@ -4451,6 +4474,9 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
 			die(_("--cached outside a repository"));
 		check_index = 1;
 	}
+	if (check_index)
+		unsafe_paths = 0;
+
 	for (i = 0; i < argc; i++) {
 		const char *arg = argv[i];
 		int fd;
diff --git a/t/t4139-apply-escape.sh b/t/t4139-apply-escape.sh
new file mode 100755
index 0000000..02b97b3
--- /dev/null
+++ b/t/t4139-apply-escape.sh
@@ -0,0 +1,141 @@
+#!/bin/sh
+
+test_description='paths written by git-apply cannot escape the working tree'
+. ./test-lib.sh
+
+# tests will try to write to ../foo, and we do not
+# want them to escape the trash directory when they
+# fail
+test_expect_success 'bump git repo one level down' '
+	mkdir inside &&
+	mv .git inside/ &&
+	cd inside
+'
+
+# $1 = name of file
+# $2 = current path to file (if different)
+mkpatch_add () {
+	rm -f "${2:-$1}" &&
+	cat <<-EOF
+	diff --git a/$1 b/$1
+	new file mode 100644
+	index 0000000..53c74cd
+	--- /dev/null
+	+++ b/$1
+	@@ -0,0 +1 @@
+	+evil
+	EOF
+}
+
+mkpatch_del () {
+	echo evil >"${2:-$1}" &&
+	cat <<-EOF
+	diff --git a/$1 b/$1
+	deleted file mode 100644
+	index 53c74cd..0000000
+	--- a/$1
+	+++ /dev/null
+	@@ -1 +0,0 @@
+	-evil
+	EOF
+}
+
+# $1 = name of file
+# $2 = content of symlink
+mkpatch_symlink () {
+	rm -f "$1" &&
+	cat <<-EOF
+	diff --git a/$1 b/$1
+	new file mode 120000
+	index 0000000..$(printf "%s" "$2" | git hash-object --stdin)
+	--- /dev/null
+	+++ b/$1
+	@@ -0,0 +1 @@
+	+$2
+	\ No newline at end of file
+	EOF
+}
+
+test_expect_success 'cannot create file containing ..' '
+	mkpatch_add ../foo >patch &&
+	test_must_fail git apply patch &&
+	test_path_is_missing ../foo
+'
+
+test_expect_success 'can create file containing .. with --unsafe-paths' '
+	mkpatch_add ../foo >patch &&
+	git apply --unsafe-paths patch &&
+	test_path_is_file ../foo
+'
+
+test_expect_success  'cannot create file containing .. (index)' '
+	mkpatch_add ../foo >patch &&
+	test_must_fail git apply --index patch &&
+	test_path_is_missing ../foo
+'
+
+test_expect_success  'cannot create file containing .. with --unsafe-paths (index)' '
+	mkpatch_add ../foo >patch &&
+	test_must_fail git apply --index --unsafe-paths patch &&
+	test_path_is_missing ../foo
+'
+
+test_expect_success 'cannot delete file containing ..' '
+	mkpatch_del ../foo >patch &&
+	test_must_fail git apply patch &&
+	test_path_is_file ../foo
+'
+
+test_expect_success 'can delete file containing .. with --unsafe-paths' '
+	mkpatch_del ../foo >patch &&
+	git apply --unsafe-paths patch &&
+	test_path_is_missing ../foo
+'
+
+test_expect_success 'cannot delete file containing .. (index)' '
+	mkpatch_del ../foo >patch &&
+	test_must_fail git apply --index patch &&
+	test_path_is_file ../foo
+'
+
+test_expect_failure SYMLINKS 'symlink escape via ..' '
+	{
+		mkpatch_symlink tmp .. &&
+		mkpatch_add tmp/foo ../foo
+	} >patch &&
+	test_must_fail git apply patch &&
+	test_path_is_missing tmp &&
+	test_path_is_missing ../foo
+'
+
+test_expect_failure SYMLINKS 'symlink escape via .. (index)' '
+	{
+		mkpatch_symlink tmp .. &&
+		mkpatch_add tmp/foo ../foo
+	} >patch &&
+	test_must_fail git apply --index patch &&
+	test_path_is_missing tmp &&
+	test_path_is_missing ../foo
+'
+
+test_expect_failure SYMLINKS 'symlink escape via absolute path' '
+	{
+		mkpatch_symlink tmp "$(pwd)" &&
+		mkpatch_add tmp/foo ../foo
+	} >patch &&
+	test_must_fail git apply patch &&
+	test_path_is_missing tmp &&
+	test_path_is_missing ../foo
+'
+
+test_expect_failure SYMLINKS 'symlink escape via absolute path (index)' '
+	{
+		mkpatch_symlink tmp "$(pwd)" &&
+		mkpatch_add tmp/foo ../foo
+	} >patch &&
+	test_must_fail git apply --index patch &&
+	test_path_is_missing tmp &&
+	test_path_is_missing ../foo
+'
+
+test_done

-- 
2.3.0-185-g073f588
Previous: Junio C HamanoNext: Junio C Hamano
Message 31 of 34 in “"git apply" safety”
  1. 0/4 "git apply" safetyJunio C Hamano, Feb 2, 2015
  2. 1/4 apply: reject input that touches outside $cwdJunio C Hamano, Feb 2, 2015
  3. Jeff KingFeb 3, 2015
  4. Jeff KingFeb 3, 2015
  5. Junio C HamanoFeb 3, 2015
  6. Jeff KingFeb 3, 2015
  7. Junio C HamanoFeb 3, 2015
  8. Jeff KingFeb 3, 2015
  9. Junio C HamanoFeb 3, 2015
  10. Jeff KingFeb 3, 2015
  11. Junio C HamanoFeb 3, 2015
  12. Torsten BögershausenFeb 3, 2015
  13. 2/4 apply: do not read from the filesystem under --indexJunio C Hamano, Feb 2, 2015
  14. 3/4 apply: do not read from beyond a symbolic linkJunio C Hamano, Feb 2, 2015
  15. Stefan BellerFeb 3, 2015
  16. Junio C HamanoFeb 3, 2015
  17. Stefan BellerFeb 3, 2015
  18. Junio C HamanoFeb 3, 2015
  19. 4/4 apply: do not touch a file beyond a symbolic linkJunio C Hamano, Feb 2, 2015
  20. Jeff KingFeb 3, 2015
  21. Junio C HamanoFeb 3, 2015
  22. Jeff KingFeb 3, 2015
  23. Junio C HamanoFeb 3, 2015
  24. Eric SunshineFeb 3, 2015
  25. 0/4 "git apply" safetyJunio C Hamano, Feb 4, 2015
  26. 1/4 apply: reject input that touches outside the working areaJunio C Hamano, Feb 4, 2015
  27. 2/4 apply: do not read from the filesystem under --indexJunio C Hamano, Feb 4, 2015
  28. 3/4 apply: do not read from beyond a symbolic linkJunio C Hamano, Feb 4, 2015
  29. 4/4 apply: do not touch a file beyond a symbolic linkJunio C Hamano, Feb 4, 2015
  30. 0/4 "git apply" safetyJunio C Hamano, Feb 10, 2015
  31. 1/4 apply: reject input that touches outside the working areaJunio C Hamano, Feb 10, 2015
  32. 2/4 apply: do not read from the filesystem under --indexJunio C Hamano, Feb 10, 2015
  33. 3/4 apply: do not read from beyond a symbolic linkJunio C Hamano, Feb 10, 2015
  34. 4/4 apply: do not touch a file beyond a symbolic linkJunio C Hamano, Feb 10, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.