Skip to content

Commit 34b6cb8

Browse files
spearcegitster
spearce
authored and
gitster
committed
http-backend: Protect GIT_PROJECT_ROOT from /../ requests
Eons ago HPA taught git-daemon how to protect itself from /../ attacks, which Junio brought back into service in d79374c ("daemon.c and path.enter_repo(): revamp path validation"). I did not carry this into git-http-backend as originally we relied only upon PATH_TRANSLATED, and assumed the HTTP server had done its access control checks to validate the resolved path was within a directory permitting access from the remote client. This would usually be sufficient to protect a server from requests for its /etc/passwd file by http://host/smart/../etc/passwd sorts of URLs. However in 917adc0 Mark Lodato added GIT_PROJECT_ROOT as an additional method of configuring the CGI. When this environment variable is used the web server does not generate the final access path and therefore may blindly pass through "/../etc/passwd" in PATH_INFO under the assumption that "/../" might have special meaning to the invoked CGI. Instead of permitting these sorts of malformed path requests, we now reject them back at the client, with an error message for the server log. This matches git-daemon behavior. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 92815b3 commit 34b6cb8

File tree

5 files changed

+86
-48
lines changed

5 files changed

+86
-48
lines changed

‎cache.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,7 @@ const char *make_relative_path(const char *abs, const char *base);
656656
int normalize_path_copy(char *dst, const char *src);
657657
int longest_ancestor_length(const char *path, const char *prefix_list);
658658
char *strip_path_suffix(const char *path, const char *suffix);
659+
int daemon_avoid_alias(const char *path);
659660

660661
/* Read and unpack a sha1 file into memory, write memory to a sha1 file */
661662
extern int sha1_object_info(const unsigned char *, unsigned long *);

‎daemon.c‎

Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -101,53 +101,6 @@ static void NORETURN daemon_die(const char *err, va_list params)
101101
exit(1);
102102
}
103103

104-
static int avoid_alias(char *p)
105-
{
106-
int sl, ndot;
107-
108-
/*
109-
* This resurrects the belts and suspenders paranoia check by HPA
110-
* done in <435560F7.4080006@zytor.com> thread, now enter_repo()
111-
* does not do getcwd() based path canonicalizations.
112-
*
113-
* sl becomes true immediately after seeing '/' and continues to
114-
* be true as long as dots continue after that without intervening
115-
* non-dot character.
116-
*/
117-
if (!p || (*p != '/' && *p != '~'))
118-
return -1;
119-
sl = 1; ndot = 0;
120-
p++;
121-
122-
while (1) {
123-
char ch = *p++;
124-
if (sl) {
125-
if (ch == '.')
126-
ndot++;
127-
else if (ch == '/') {
128-
if (ndot < 3)
129-
/* reject //, /./ and /../ */
130-
return -1;
131-
ndot = 0;
132-
}
133-
else if (ch == 0) {
134-
if (0 < ndot && ndot < 3)
135-
/* reject /.$ and /..$ */
136-
return -1;
137-
return 0;
138-
}
139-
else
140-
sl = ndot = 0;
141-
}
142-
else if (ch == 0)
143-
return 0;
144-
else if (ch == '/') {
145-
sl = 1;
146-
ndot = 0;
147-
}
148-
}
149-
}
150-
151104
static char *path_ok(char *directory)
152105
{
153106
static char rpath[PATH_MAX];
@@ -157,7 +110,7 @@ static char *path_ok(char *directory)
157110

158111
dir = directory;
159112

160-
if (avoid_alias(dir)) {
113+
if (daemon_avoid_alias(dir)) {
161114
logerror("'%s': aliased", dir);
162115
return NULL;
163116
}

‎http-backend.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,13 @@ static char* getdir(void)
559559
if (root && *root) {
560560
if (!pathinfo || !*pathinfo)
561561
die("GIT_PROJECT_ROOT is set but PATH_INFO is not");
562+
if (daemon_avoid_alias(pathinfo))
563+
die("'%s': aliased", pathinfo);
562564
strbuf_addstr(&buf, root);
565+
if (buf.buf[buf.len - 1] != '/')
566+
strbuf_addch(&buf, '/');
567+
if (pathinfo[0] == '/')
568+
pathinfo++;
563569
strbuf_addstr(&buf, pathinfo);
564570
return strbuf_detach(&buf, NULL);
565571
} else if (path && *path) {

‎path.c‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,3 +564,50 @@ char *strip_path_suffix(const char *path, const char *suffix)
564564
return NULL;
565565
return xstrndup(path, chomp_trailing_dir_sep(path, path_len));
566566
}
567+
568+
int daemon_avoid_alias(const char *p)
569+
{
570+
int sl, ndot;
571+
572+
/*
573+
* This resurrects the belts and suspenders paranoia check by HPA
574+
* done in <435560F7.4080006@zytor.com> thread, now enter_repo()
575+
* does not do getcwd() based path canonicalizations.
576+
*
577+
* sl becomes true immediately after seeing '/' and continues to
578+
* be true as long as dots continue after that without intervening
579+
* non-dot character.
580+
*/
581+
if (!p || (*p != '/' && *p != '~'))
582+
return -1;
583+
sl = 1; ndot = 0;
584+
p++;
585+
586+
while (1) {
587+
char ch = *p++;
588+
if (sl) {
589+
if (ch == '.')
590+
ndot++;
591+
else if (ch == '/') {
592+
if (ndot < 3)
593+
/* reject //, /./ and /../ */
594+
return -1;
595+
ndot = 0;
596+
}
597+
else if (ch == 0) {
598+
if (0 < ndot && ndot < 3)
599+
/* reject /.$ and /..$ */
600+
return -1;
601+
return 0;
602+
}
603+
else
604+
sl = ndot = 0;
605+
}
606+
else if (ch == 0)
607+
return 0;
608+
else if (ch == '/') {
609+
sl = 1;
610+
ndot = 0;
611+
}
612+
}
613+
}

‎t/t5560-http-backend.sh‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,37 @@ test_expect_success 'http.receivepack false' '
146146
POST git-receive-pack 0000 "403 Forbidden"
147147
'
148148

149+
run_backend() {
150+
REQUEST_METHOD=GET \
151+
GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
152+
PATH_INFO="$2" \
153+
git http-backend >act.out 2>act.err
154+
}
155+
156+
path_info() {
157+
if test $1 = 0; then
158+
run_backend "$2"
159+
else
160+
test_must_fail run_backend "$2" &&
161+
echo "fatal: '$2': aliased" >exp.err &&
162+
test_cmp exp.err act.err
163+
fi
164+
}
165+
166+
test_expect_success 'http-backend blocks bad PATH_INFO' '
167+
config http.getanyfile true &&
168+
169+
run_backend 0 /repo.git/HEAD &&
170+
171+
run_backend 1 /repo.git/../HEAD &&
172+
run_backend 1 /../etc/passwd &&
173+
run_backend 1 ../etc/passwd &&
174+
run_backend 1 /etc//passwd &&
175+
run_backend 1 /etc/./passwd &&
176+
run_backend 1 /etc/.../passwd &&
177+
run_backend 1 //domain/data.txt
178+
'
179+
149180
cat >exp <<EOF
150181
151182
### refs/heads/master

0 commit comments

Comments
 (0)