apply: reject input that touches outside the working area

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>

Junio C Hamano committed Jan 29, 2015 at 15:35 UTC c536c0755f6450b7bcce499cfda171f8c6d1e593
3 files changed +178 -1
Documentation/git-apply.txt
+11 -1
@@ -16,7 +16,7 @@ SYNOPSIS
16 [--ignore-space-change | --ignore-whitespace ]
17 [--whitespace=(nowarn|warn|fix|error|error-all)]
18 [--exclude=<path>] [--include=<path>] [--directory=<root>]
19 - [--verbose] [<patch>...]
19 + [--verbose] [--unsafe-paths] [<patch>...]
20
21 DESCRIPTION
22 -----------
@@ -229,6 +229,16 @@ For example, a patch that talks about updating `a/git-gui.sh` to `b/git-gui.sh`
229 can be applied to the file in the working tree `modules/git-gui/git-gui.sh` by
230 running `git apply --directory=modules/git-gui`.
231
232 +--unsafe-paths::
233 + By default, a patch that affects outside the working area
234 + (either a Git controlled working tree, or the current working
235 + directory when "git apply" is used as a replacement of GNU
236 + patch) is rejected as a mistake (or a mischief).
237 ++
238 +When `git apply` is used as a "better GNU patch", the user can pass
239 +the `--unsafe-paths` option to override this safety check. This option
240 +has no effect when `--index` or `--cached` is in use.
241 +
242 Configuration
243 -------------
244
builtin/apply.c
+26
@@ -50,6 +50,7 @@ static int apply_verbosely;
50 static int allow_overlap;
51 static int no_add;
52 static int threeway;
53 +static int unsafe_paths;
54 static const char *fake_ancestor;
55 static int line_termination = '\n';
56 static unsigned int p_context = UINT_MAX;
@@ -3483,6 +3484,23 @@ static int check_to_create(const char *new_name, int ok_if_exists)
3484 return 0;
3485 }
3486
3487 +static void die_on_unsafe_path(struct patch *patch)
3488 +{
3489 + const char *old_name = NULL;
3490 + const char *new_name = NULL;
3491 + if (patch->is_delete)
3492 + old_name = patch->old_name;
3493 + else if (!patch->is_new && !patch->is_copy)
3494 + old_name = patch->old_name;
3495 + if (!patch->is_delete)
3496 + new_name = patch->new_name;
3497 +
3498 + if (old_name && !verify_path(old_name))
3499 + die(_("invalid path '%s'"), old_name);
3500 + if (new_name && !verify_path(new_name))
3501 + die(_("invalid path '%s'"), new_name);
3502 +}
3503 +
3504 /*
3505 * Check and apply the patch in-core; leave the result in patch->result
3506 * for the caller to write it out to the final destination.
@@ -3570,6 +3588,9 @@ static int check_patch(struct patch *patch)
3588 }
3589 }
3590
3591 + if (!unsafe_paths)
3592 + die_on_unsafe_path(patch);
3593 +
3594 if (apply_data(patch, &st, ce) < 0)
3595 return error(_("%s: patch does not apply"), name);
3596 patch->rejected = 0;
@@ -4379,6 +4400,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
4400 N_("make sure the patch is applicable to the current index")),
4401 OPT_BOOL(0, "cached", &cached,
4402 N_("apply a patch without touching the working tree")),
4403 + OPT_BOOL(0, "unsafe-paths", &unsafe_paths,
4404 + N_("accept a patch that touches outside the working area")),
4405 OPT_BOOL(0, "apply", &force_apply,
4406 N_("also apply the patch (use with --stat/--summary/--check)")),
4407 OPT_BOOL('3', "3way", &threeway,
@@ -4451,6 +4474,9 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
4474 die(_("--cached outside a repository"));
4475 check_index = 1;
4476 }
4477 + if (check_index)
4478 + unsafe_paths = 0;
4479 +
4480 for (i = 0; i < argc; i++) {
4481 const char *arg = argv[i];
4482 int fd;
t/t4139-apply-escape.sh new
+141
@@ -0,0 +1,141 @@
1 +#!/bin/sh
2 +
3 +test_description='paths written by git-apply cannot escape the working tree'
4 +. ./test-lib.sh
5 +
6 +# tests will try to write to ../foo, and we do not
7 +# want them to escape the trash directory when they
8 +# fail
9 +test_expect_success 'bump git repo one level down' '
10 + mkdir inside &&
11 + mv .git inside/ &&
12 + cd inside
13 +'
14 +
15 +# $1 = name of file
16 +# $2 = current path to file (if different)
17 +mkpatch_add () {
18 + rm -f "${2:-$1}" &&
19 + cat <<-EOF
20 + diff --git a/$1 b/$1
21 + new file mode 100644
22 + index 0000000..53c74cd
23 + --- /dev/null
24 + +++ b/$1
25 + @@ -0,0 +1 @@
26 + +evil
27 + EOF
28 +}
29 +
30 +mkpatch_del () {
31 + echo evil >"${2:-$1}" &&
32 + cat <<-EOF
33 + diff --git a/$1 b/$1
34 + deleted file mode 100644
35 + index 53c74cd..0000000
36 + --- a/$1
37 + +++ /dev/null
38 + @@ -1 +0,0 @@
39 + -evil
40 + EOF
41 +}
42 +
43 +# $1 = name of file
44 +# $2 = content of symlink
45 +mkpatch_symlink () {
46 + rm -f "$1" &&
47 + cat <<-EOF
48 + diff --git a/$1 b/$1
49 + new file mode 120000
50 + index 0000000..$(printf "%s" "$2" | git hash-object --stdin)
51 + --- /dev/null
52 + +++ b/$1
53 + @@ -0,0 +1 @@
54 + +$2
55 + \ No newline at end of file
56 + EOF
57 +}
58 +
59 +test_expect_success 'cannot create file containing ..' '
60 + mkpatch_add ../foo >patch &&
61 + test_must_fail git apply patch &&
62 + test_path_is_missing ../foo
63 +'
64 +
65 +test_expect_success 'can create file containing .. with --unsafe-paths' '
66 + mkpatch_add ../foo >patch &&
67 + git apply --unsafe-paths patch &&
68 + test_path_is_file ../foo
69 +'
70 +
71 +test_expect_success 'cannot create file containing .. (index)' '
72 + mkpatch_add ../foo >patch &&
73 + test_must_fail git apply --index patch &&
74 + test_path_is_missing ../foo
75 +'
76 +
77 +test_expect_success 'cannot create file containing .. with --unsafe-paths (index)' '
78 + mkpatch_add ../foo >patch &&
79 + test_must_fail git apply --index --unsafe-paths patch &&
80 + test_path_is_missing ../foo
81 +'
82 +
83 +test_expect_success 'cannot delete file containing ..' '
84 + mkpatch_del ../foo >patch &&
85 + test_must_fail git apply patch &&
86 + test_path_is_file ../foo
87 +'
88 +
89 +test_expect_success 'can delete file containing .. with --unsafe-paths' '
90 + mkpatch_del ../foo >patch &&
91 + git apply --unsafe-paths patch &&
92 + test_path_is_missing ../foo
93 +'
94 +
95 +test_expect_success 'cannot delete file containing .. (index)' '
96 + mkpatch_del ../foo >patch &&
97 + test_must_fail git apply --index patch &&
98 + test_path_is_file ../foo
99 +'
100 +
101 +test_expect_failure SYMLINKS 'symlink escape via ..' '
102 + {
103 + mkpatch_symlink tmp .. &&
104 + mkpatch_add tmp/foo ../foo
105 + } >patch &&
106 + test_must_fail git apply patch &&
107 + test_path_is_missing tmp &&
108 + test_path_is_missing ../foo
109 +'
110 +
111 +test_expect_failure SYMLINKS 'symlink escape via .. (index)' '
112 + {
113 + mkpatch_symlink tmp .. &&
114 + mkpatch_add tmp/foo ../foo
115 + } >patch &&
116 + test_must_fail git apply --index patch &&
117 + test_path_is_missing tmp &&
118 + test_path_is_missing ../foo
119 +'
120 +
121 +test_expect_failure SYMLINKS 'symlink escape via absolute path' '
122 + {
123 + mkpatch_symlink tmp "$(pwd)" &&
124 + mkpatch_add tmp/foo ../foo
125 + } >patch &&
126 + test_must_fail git apply patch &&
127 + test_path_is_missing tmp &&
128 + test_path_is_missing ../foo
129 +'
130 +
131 +test_expect_failure SYMLINKS 'symlink escape via absolute path (index)' '
132 + {
133 + mkpatch_symlink tmp "$(pwd)" &&
134 + mkpatch_add tmp/foo ../foo
135 + } >patch &&
136 + test_must_fail git apply --index patch &&
137 + test_path_is_missing tmp &&
138 + test_path_is_missing ../foo
139 +'
140 +
141 +test_done