Skip to content

Commit c536c07

Browse files
gitstergitster
committed
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 <[email protected]>
1 parent 3d8a54e commit c536c07

File tree

3 files changed

+178
-1
lines changed

3 files changed

+178
-1
lines changed

‎Documentation/git-apply.txt

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ SYNOPSIS
1616
[--ignore-space-change | --ignore-whitespace ]
1717
[--whitespace=(nowarn|warn|fix|error|error-all)]
1818
[--exclude=<path>] [--include=<path>] [--directory=<root>]
19-
[--verbose] [<patch>...]
19+
[--verbose] [--unsafe-paths] [<patch>...]
2020

2121
DESCRIPTION
2222
-----------
@@ -229,6 +229,16 @@ For example, a patch that talks about updating `a/git-gui.sh` to `b/git-gui.sh`
229229
can be applied to the file in the working tree `modules/git-gui/git-gui.sh` by
230230
running `git apply --directory=modules/git-gui`.
231231

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+
232242
Configuration
233243
-------------
234244

‎builtin/apply.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ static int apply_verbosely;
5050
static int allow_overlap;
5151
static int no_add;
5252
static int threeway;
53+
static int unsafe_paths;
5354
static const char *fake_ancestor;
5455
static int line_termination = '\n';
5556
static unsigned int p_context = UINT_MAX;
@@ -3483,6 +3484,23 @@ static int check_to_create(const char *new_name, int ok_if_exists)
34833484
return 0;
34843485
}
34853486

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+
34863504
/*
34873505
* Check and apply the patch in-core; leave the result in patch->result
34883506
* for the caller to write it out to the final destination.
@@ -3570,6 +3588,9 @@ static int check_patch(struct patch *patch)
35703588
}
35713589
}
35723590

3591+
if (!unsafe_paths)
3592+
die_on_unsafe_path(patch);
3593+
35733594
if (apply_data(patch, &st, ce) < 0)
35743595
return error(_("%s: patch does not apply"), name);
35753596
patch->rejected = 0;
@@ -4379,6 +4400,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
43794400
N_("make sure the patch is applicable to the current index")),
43804401
OPT_BOOL(0, "cached", &cached,
43814402
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")),
43824405
OPT_BOOL(0, "apply", &force_apply,
43834406
N_("also apply the patch (use with --stat/--summary/--check)")),
43844407
OPT_BOOL('3', "3way", &threeway,
@@ -4451,6 +4474,9 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
44514474
die(_("--cached outside a repository"));
44524475
check_index = 1;
44534476
}
4477+
if (check_index)
4478+
unsafe_paths = 0;
4479+
44544480
for (i = 0; i < argc; i++) {
44554481
const char *arg = argv[i];
44564482
int fd;

‎t/t4139-apply-escape.sh

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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

0 commit comments

Comments
 (0)