Skip to content

Commit d089eba

Browse files
gitstergitster
committed
setup: sanitize absolute and funny paths in get_pathspec()
The prefix_path() function called from get_pathspec() is responsible for translating list of user-supplied pathspecs to list of pathspecs that is relative to the root of the work tree. When working inside a subdirectory, the user-supplied pathspecs are taken to be relative to the current subdirectory. Among special path components in pathspecs, we used to accept and interpret only "." ("the directory", meaning a no-op) and ".." ("up one level") at the beginning. Everything else was passed through as-is. For example, if you are in Documentation/ directory of the project, you can name Documentation/howto/maintain-git.txt as: howto/maintain-git.txt ../Documentation/howto/maitain-git.txt ../././Documentation/howto/maitain-git.txt but not as: howto/./maintain-git.txt $(pwd)/howto/maintain-git.txt This patch updates prefix_path() in several ways: - If the pathspec is not absolute, prefix (i.e. the current subdirectory relative to the root of the work tree, with terminating slash, if not empty) and the pathspec is concatenated first and used in the next step. Otherwise, that absolute pathspec is used in the next step. - Then special path components "." (no-op) and ".." (up one level) are interpreted to simplify the path. It is an error to have too many ".." to cause the intermediate result to step outside of the input to this step. - If the original pathspec was not absolute, the result from the previous step is the resulting "sanitized" pathspec. Otherwise, the result from the previous step is still absolute, and it is an error if it does not begin with the directory that corresponds to the root of the work tree. The directory is stripped away from the result and is returned. - In any case, the resulting pathspec in the array get_pathspec() returns omit the ones that caused errors. With this patch, the last two examples also behave as expected. Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 7a2078b commit d089eba

File tree

4 files changed

+245
-45
lines changed

4 files changed

+245
-45
lines changed

‎builtin-ls-files.c‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,8 +572,17 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
572572
pathspec = get_pathspec(prefix, argv + i);
573573

574574
/* Verify that the pathspec matches the prefix */
575-
if (pathspec)
575+
if (pathspec) {
576+
if (argc != i) {
577+
int cnt;
578+
for (cnt = 0; pathspec[cnt]; cnt++)
579+
;
580+
if (cnt != (argc - i))
581+
exit(1); /* error message already given */
582+
}
576583
prefix = verify_pathspec(prefix);
584+
} else if (argc != i)
585+
exit(1); /* error message already given */
577586

578587
/* Treat unmatching pathspec elements as errors */
579588
if (pathspec && error_unmatch) {

‎builtin-mv.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,15 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
164164
}
165165

166166
dst = add_slash(dst);
167-
dst_len = strlen(dst) - 1;
167+
dst_len = strlen(dst);
168168

169169
for (j = 0; j < last - first; j++) {
170170
const char *path =
171171
active_cache[first + j]->name;
172172
source[argc + j] = path;
173173
destination[argc + j] =
174174
prefix_path(dst, dst_len,
175-
path + length);
175+
path + length + 1);
176176
modes[argc + j] = INDEX;
177177
}
178178
argc += last - first;

‎setup.c‎

Lines changed: 116 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -4,51 +4,118 @@
44
static int inside_git_dir = -1;
55
static int inside_work_tree = -1;
66

7-
const char *prefix_path(const char *prefix, int len, const char *path)
7+
static int sanitary_path_copy(char *dst, const char *src)
88
{
9-
const char *orig = path;
9+
char *dst0 = dst;
10+
11+
if (*src == '/') {
12+
*dst++ = '/';
13+
while (*src == '/')
14+
src++;
15+
}
16+
1017
for (;;) {
11-
char c;
12-
if (*path != '.')
13-
break;
14-
c = path[1];
15-
/* "." */
16-
if (!c) {
17-
path++;
18-
break;
18+
char c = *src;
19+
20+
/*
21+
* A path component that begins with . could be
22+
* special:
23+
* (1) "." and ends -- ignore and terminate.
24+
* (2) "./" -- ignore them, eat slash and continue.
25+
* (3) ".." and ends -- strip one and terminate.
26+
* (4) "../" -- strip one, eat slash and continue.
27+
*/
28+
if (c == '.') {
29+
switch (src[1]) {
30+
case '\0':
31+
/* (1) */
32+
src++;
33+
break;
34+
case '/':
35+
/* (2) */
36+
src += 2;
37+
while (*src == '/')
38+
src++;
39+
continue;
40+
case '.':
41+
switch (src[2]) {
42+
case '\0':
43+
/* (3) */
44+
src += 2;
45+
goto up_one;
46+
case '/':
47+
/* (4) */
48+
src += 3;
49+
while (*src == '/')
50+
src++;
51+
goto up_one;
52+
}
53+
}
1954
}
20-
/* "./" */
55+
56+
/* copy up to the next '/', and eat all '/' */
57+
while ((c = *src++) != '\0' && c != '/')
58+
*dst++ = c;
2159
if (c == '/') {
22-
path += 2;
23-
continue;
24-
}
25-
if (c != '.')
60+
*dst++ = c;
61+
while (c == '/')
62+
c = *src++;
63+
src--;
64+
} else if (!c)
2665
break;
27-
c = path[2];
28-
if (!c)
29-
path += 2;
30-
else if (c == '/')
31-
path += 3;
32-
else
33-
break;
34-
/* ".." and "../" */
35-
/* Remove last component of the prefix */
36-
do {
37-
if (!len)
38-
die("'%s' is outside repository", orig);
39-
len--;
40-
} while (len && prefix[len-1] != '/');
4166
continue;
67+
68+
up_one:
69+
/*
70+
* dst0..dst is prefix portion, and dst[-1] is '/';
71+
* go up one level.
72+
*/
73+
dst -= 2; /* go past trailing '/' if any */
74+
if (dst < dst0)
75+
return -1;
76+
while (1) {
77+
if (dst <= dst0)
78+
break;
79+
c = *dst--;
80+
if (c == '/') {
81+
dst += 2;
82+
break;
83+
}
84+
}
4285
}
43-
if (len) {
44-
int speclen = strlen(path);
45-
char *n = xmalloc(speclen + len + 1);
86+
*dst = '\0';
87+
return 0;
88+
}
4689

47-
memcpy(n, prefix, len);
48-
memcpy(n + len, path, speclen+1);
49-
path = n;
90+
const char *prefix_path(const char *prefix, int len, const char *path)
91+
{
92+
const char *orig = path;
93+
char *sanitized = xmalloc(len + strlen(path) + 1);
94+
if (*orig == '/')
95+
strcpy(sanitized, path);
96+
else {
97+
if (len)
98+
memcpy(sanitized, prefix, len);
99+
strcpy(sanitized + len, path);
50100
}
51-
return path;
101+
if (sanitary_path_copy(sanitized, sanitized))
102+
goto error_out;
103+
if (*orig == '/') {
104+
const char *work_tree = get_git_work_tree();
105+
size_t len = strlen(work_tree);
106+
size_t total = strlen(sanitized) + 1;
107+
if (strncmp(sanitized, work_tree, len) ||
108+
(sanitized[len] != '\0' && sanitized[len] != '/')) {
109+
error_out:
110+
error("'%s' is outside repository", orig);
111+
free(sanitized);
112+
return NULL;
113+
}
114+
if (sanitized[len] == '/')
115+
len++;
116+
memmove(sanitized, sanitized + len, total - len);
117+
}
118+
return sanitized;
52119
}
53120

54121
/*
@@ -114,7 +181,7 @@ void verify_non_filename(const char *prefix, const char *arg)
114181
const char **get_pathspec(const char *prefix, const char **pathspec)
115182
{
116183
const char *entry = *pathspec;
117-
const char **p;
184+
const char **src, **dst;
118185
int prefixlen;
119186

120187
if (!prefix && !entry)
@@ -128,12 +195,19 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
128195
}
129196

130197
/* Otherwise we have to re-write the entries.. */
131-
p = pathspec;
198+
src = pathspec;
199+
dst = pathspec;
132200
prefixlen = prefix ? strlen(prefix) : 0;
133-
do {
134-
*p = prefix_path(prefix, prefixlen, entry);
135-
} while ((entry = *++p) != NULL);
136-
return (const char **) pathspec;
201+
while (*src) {
202+
const char *p = prefix_path(prefix, prefixlen, *src);
203+
if (p)
204+
*(dst++) = p;
205+
src++;
206+
}
207+
*dst = NULL;
208+
if (!*pathspec)
209+
return NULL;
210+
return pathspec;
137211
}
138212

139213
/*

‎t/t7010-setup.sh‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
#!/bin/sh
2+
3+
test_description='setup taking and sanitizing funny paths'
4+
5+
. ./test-lib.sh
6+
7+
test_expect_success setup '
8+
9+
mkdir -p a/b/c a/e &&
10+
D=$(pwd) &&
11+
>a/b/c/d &&
12+
>a/e/f
13+
14+
'
15+
16+
test_expect_success 'git add (absolute)' '
17+
18+
git add "$D/a/b/c/d" &&
19+
git ls-files >current &&
20+
echo a/b/c/d >expect &&
21+
diff -u expect current
22+
23+
'
24+
25+
26+
test_expect_success 'git add (funny relative)' '
27+
28+
rm -f .git/index &&
29+
(
30+
cd a/b &&
31+
git add "../e/./f"
32+
) &&
33+
git ls-files >current &&
34+
echo a/e/f >expect &&
35+
diff -u expect current
36+
37+
'
38+
39+
test_expect_success 'git rm (absolute)' '
40+
41+
rm -f .git/index &&
42+
git add a &&
43+
git rm -f --cached "$D/a/b/c/d" &&
44+
git ls-files >current &&
45+
echo a/e/f >expect &&
46+
diff -u expect current
47+
48+
'
49+
50+
test_expect_success 'git rm (funny relative)' '
51+
52+
rm -f .git/index &&
53+
git add a &&
54+
(
55+
cd a/b &&
56+
git rm -f --cached "../e/./f"
57+
) &&
58+
git ls-files >current &&
59+
echo a/b/c/d >expect &&
60+
diff -u expect current
61+
62+
'
63+
64+
test_expect_success 'git ls-files (absolute)' '
65+
66+
rm -f .git/index &&
67+
git add a &&
68+
git ls-files "$D/a/e/../b" >current &&
69+
echo a/b/c/d >expect &&
70+
diff -u expect current
71+
72+
'
73+
74+
test_expect_success 'git ls-files (relative #1)' '
75+
76+
rm -f .git/index &&
77+
git add a &&
78+
(
79+
cd a/b &&
80+
git ls-files "../b/c"
81+
) >current &&
82+
echo c/d >expect &&
83+
diff -u expect current
84+
85+
'
86+
87+
test_expect_success 'git ls-files (relative #2)' '
88+
89+
rm -f .git/index &&
90+
git add a &&
91+
(
92+
cd a/b &&
93+
git ls-files --full-name "../e/f"
94+
) >current &&
95+
echo a/e/f >expect &&
96+
diff -u expect current
97+
98+
'
99+
100+
test_expect_success 'git ls-files (relative #3)' '
101+
102+
rm -f .git/index &&
103+
git add a &&
104+
(
105+
cd a/b &&
106+
if git ls-files "../e/f"
107+
then
108+
echo Gaah, should have failed
109+
exit 1
110+
else
111+
: happy
112+
fi
113+
)
114+
115+
'
116+
117+
test_done

0 commit comments

Comments
 (0)