[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <xmqqzj91cfnl.fsf_-_@gitster.dls.corp.google.com>
Date: Thu, 29 Jan 2015 15:48:14 -0800
From: Junio C Hamano <gitster@...ox.com>
To: Git Mailing List <git@...r.kernel.org>
Cc: Jeff King <peff@...f.net>, Josh Boyer <jwboyer@...oraproject.org>,
"Linux-Kernel\@Vger. Kernel. Org" <linux-kernel@...r.kernel.org>,
twaugh@...hat.com, Linus Torvalds <torvalds@...ux-foundation.org>
Subject: [PATCH 2/1] apply: reject input that touches outside $cwd
By default, a patch that affects outside the working area is
rejected as a mistake; 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 `--allow-uplevel`
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@...ox.com>
---
* Meant to apply on top of the previous one, but these two are
about separate and orthogonal issues.
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..20c3a6f 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] [--allow-uplevel] [<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`.
+--allow-uplevel::
+ By default, a patch that affects outside the working area is
+ rejected as a mistake; 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 `--allow-uplevel`
+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 dcb44fb..ce5a594 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 allow_uplevel;
static const char *fake_ancestor;
static int line_termination = '\n';
static unsigned int p_context = UINT_MAX;
@@ -3523,6 +3524,23 @@ symlink_found:
}
+static void die_on_uplevel_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.
@@ -3614,6 +3632,9 @@ static int check_patch(struct patch *patch)
return error(_("affected file '%s' is beyond a symbolic link"),
patch->new_name);
+ if (!allow_uplevel)
+ die_on_uplevel_path(patch);
+
if (apply_data(patch, &st, ce) < 0)
return error(_("%s: patch does not apply"), name);
patch->rejected = 0;
@@ -4423,6 +4444,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, "allow-uplevel", &allow_uplevel,
+ N_("accept a patch to touch outside the current directory")),
OPT_BOOL(0, "apply", &force_apply,
N_("also apply the patch (use with --stat/--summary/--check)")),
OPT_BOOL('3', "3way", &threeway,
@@ -4495,6 +4518,9 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
die(_("--cached outside a repository"));
check_index = 1;
}
+ if (check_index)
+ allow_uplevel = 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..39de838
--- /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 --allow-uplevel' '
+ mkpatch_add ../foo >patch &&
+ git apply --allow-uplevel 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 --allow-uplevel (index)' '
+ mkpatch_add ../foo >patch &&
+ test_must_fail git apply --index --allow-uplevel 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 --allow-uplevel' '
+ mkpatch_del ../foo >patch &&
+ git apply --allow-uplevel 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_success '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_success '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_success '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-158-g17413e7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists