All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH v2 1/4] apply: reject input that touches outside $cwd
Date: Mon,  2 Feb 2015 15:27:27 -0800	[thread overview]
Message-ID: <1422919650-13346-2-git-send-email-gitster@pobox.com> (raw)
In-Reply-To: <1422919650-13346-1-git-send-email-gitster@pobox.com>

By default, a patch that affects outside the working area is
rejected as a mistake (or a mischief); Git itself does not create
such a patch, unless the user bends backwards and specifies a
non-standard prefix to "git diff" and friends.

When `git apply` is used without either `--index` or `--cached`
option as a "better GNU patch", the user can pass `--unsafe-paths`
option to override this safety check.  This cannot be used to escape
outside the working tree when using `--index` or `--cached` to apply
the patch to the index.

The new test was stolen from Jeff King with slight enhancements.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-apply.txt |  14 ++++-
 builtin/apply.c             |  26 +++++++++
 t/t4139-apply-escape.sh     | 137 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 176 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..a6e83a9 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,18 @@ 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 is
+	rejected as a mistake (or a mischief); Git itself never
+	creates such a patch unless the user bends backwards and
+	specifies nonstandard prefix to "git diff" and friends.
++
+When `git apply` is used without either `--index` or `--cached`
+option as a "better GNU patch", the user can pass `--unsafe-paths`
+option to override this safety check.  This cannot be used to escape
+outside the working tree when using `--index` or `--cached` to apply
+the patch to the index.
+
 Configuration
 -------------
 
diff --git a/builtin/apply.c b/builtin/apply.c
index ef32e4f..c751ddf 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 to touch 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..5e67179
--- /dev/null
+++ b/t/t4139-apply-escape.sh
@@ -0,0 +1,137 @@
+#!/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 add file containing ..' '
+	mkpatch_add ../foo >patch &&
+	test_must_fail git apply patch &&
+	test_path_is_missing ../foo
+'
+
+test_expect_success 'can add file containing .. with --unsafe-paths' '
+	mkpatch_add ../foo >patch &&
+	git apply --unsafe-paths patch &&
+	test_path_is_file ../foo
+'
+
+test_expect_success  'cannot add file containing .. (index)' '
+	mkpatch_add ../foo >patch &&
+	test_must_fail git apply --index patch &&
+	test_path_is_missing ../foo
+'
+
+test_expect_success  'cannot add 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 del file containing ..' '
+	mkpatch_del ../foo >patch &&
+	test_must_fail git apply patch &&
+	test_path_is_file ../foo
+'
+
+test_expect_success 'can del file containing .. with --unsafe-paths' '
+	mkpatch_del ../foo >patch &&
+	git apply --unsafe-paths patch &&
+	test_path_is_missing ../foo
+'
+
+test_expect_success 'cannot del file containing .. (index)' '
+	mkpatch_del ../foo >patch &&
+	test_must_fail git apply --index patch &&
+	test_path_is_file ../foo
+'
+
+test_expect_failure 'symlink escape via ..' '
+	{
+		mkpatch_symlink tmp .. &&
+		mkpatch_add tmp/foo ../foo
+	} >patch &&
+	test_must_fail git apply patch &&
+	test_path_is_missing ../foo
+'
+
+test_expect_failure 'symlink escape via .. (index)' '
+	{
+		mkpatch_symlink tmp .. &&
+		mkpatch_add tmp/foo ../foo
+	} >patch &&
+	test_must_fail git apply --index patch &&
+	test_path_is_missing ../foo
+'
+
+test_expect_failure '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 ../foo
+'
+
+test_expect_success '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 ../foo
+'
+
+test_done
-- 
2.3.0-rc2-164-g799cdce

  reply	other threads:[~2015-02-02 23:27 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-02 23:27 [PATCH v2 0/4] "git apply" safety Junio C Hamano
2015-02-02 23:27 ` Junio C Hamano [this message]
2015-02-03  0:45   ` [PATCH v2 1/4] apply: reject input that touches outside $cwd Jeff King
2015-02-03  0:50   ` Jeff King
2015-02-03 20:23     ` Junio C Hamano
2015-02-03 21:01       ` Jeff King
2015-02-03 21:23         ` Junio C Hamano
2015-02-03 21:24           ` Jeff King
2015-02-03 21:40             ` Junio C Hamano
2015-02-03 21:50               ` Jeff King
2015-02-03 22:11                 ` Junio C Hamano
2015-02-03  5:56   ` Torsten Bögershausen
2015-02-02 23:27 ` [PATCH v2 2/4] apply: do not read from the filesystem under --index Junio C Hamano
2015-02-02 23:27 ` [PATCH v2 3/4] apply: do not read from beyond a symbolic link Junio C Hamano
2015-02-03  0:08   ` Stefan Beller
2015-02-03 19:37     ` Junio C Hamano
2015-02-03 19:44       ` Stefan Beller
2015-02-03 20:31         ` Junio C Hamano
2015-02-02 23:27 ` [PATCH v2 4/4] apply: do not touch a file " Junio C Hamano
2015-02-03  1:11   ` Jeff King
2015-02-03  1:56     ` Junio C Hamano
2015-02-03  2:04       ` Jeff King
2015-02-03 21:01     ` Junio C Hamano
2015-02-03 23:40       ` Eric Sunshine
2015-02-04  0:44 ` [PATCH v3 0/4] "git apply" safety Junio C Hamano
2015-02-04  0:44   ` [PATCH v3 1/4] apply: reject input that touches outside the working area Junio C Hamano
2015-02-04  0:44   ` [PATCH v3 2/4] apply: do not read from the filesystem under --index Junio C Hamano
2015-02-04  0:44   ` [PATCH v3 3/4] apply: do not read from beyond a symbolic link Junio C Hamano
2015-02-04  0:44   ` [PATCH v3 4/4] apply: do not touch a file " Junio C Hamano
2015-02-10 22:36   ` [PATCH v4 0/4] "git apply" safety Junio C Hamano
2015-02-10 22:36     ` [PATCH v4 1/4] apply: reject input that touches outside the working area Junio C Hamano
2015-02-10 22:36     ` [PATCH v4 2/4] apply: do not read from the filesystem under --index Junio C Hamano
2015-02-10 22:36     ` [PATCH v4 3/4] apply: do not read from beyond a symbolic link Junio C Hamano
2015-02-10 22:36     ` [PATCH v4 4/4] apply: do not touch a file " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1422919650-13346-2-git-send-email-gitster@pobox.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.