Skip to content

Commit c722ba4

Browse files
committed
Merge branch 'jk/daemon-interpolate' into maint
The "interpolated-path" option of "git daemon" inserted any string client declared on the "host=" capability request without checking. Sanitize and limit %H and %CH to a saner and a valid DNS name. * jk/daemon-interpolate: daemon: sanitize incoming virtual hostname t5570: test git-daemon's --interpolated-path option git_connect: let user override virtual-host we send to daemon
2 parents 1165ae6 + b485373 commit c722ba4

File tree

3 files changed

+83
-6
lines changed

3 files changed

+83
-6
lines changed

connect.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -669,10 +669,20 @@ struct child_process *git_connect(int fd[2], const char *url,
669669
printf("Diag: path=%s\n", path ? path : "NULL");
670670
conn = NULL;
671671
} else if (protocol == PROTO_GIT) {
672+
/*
673+
* Set up virtual host information based on where we will
674+
* connect, unless the user has overridden us in
675+
* the environment.
676+
*/
677+
char *target_host = getenv("GIT_OVERRIDE_VIRTUAL_HOST");
678+
if (target_host)
679+
target_host = xstrdup(target_host);
680+
else
681+
target_host = xstrdup(hostandport);
682+
672683
/* These underlying connection commands die() if they
673684
* cannot connect.
674685
*/
675-
char *target_host = xstrdup(hostandport);
676686
if (git_use_proxy(hostandport))
677687
conn = git_proxy_connect(fd, hostandport);
678688
else

daemon.c

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,45 @@ static void parse_host_and_port(char *hostport, char **host,
484484
}
485485
}
486486

487+
/*
488+
* Sanitize a string from the client so that it's OK to be inserted into a
489+
* filesystem path. Specifically, we disallow slashes, runs of "..", and
490+
* trailing and leading dots, which means that the client cannot escape
491+
* our base path via ".." traversal.
492+
*/
493+
static void sanitize_client_strbuf(struct strbuf *out, const char *in)
494+
{
495+
for (; *in; in++) {
496+
if (*in == '/')
497+
continue;
498+
if (*in == '.' && (!out->len || out->buf[out->len - 1] == '.'))
499+
continue;
500+
strbuf_addch(out, *in);
501+
}
502+
503+
while (out->len && out->buf[out->len - 1] == '.')
504+
strbuf_setlen(out, out->len - 1);
505+
}
506+
507+
static char *sanitize_client(const char *in)
508+
{
509+
struct strbuf out = STRBUF_INIT;
510+
sanitize_client_strbuf(&out, in);
511+
return strbuf_detach(&out, NULL);
512+
}
513+
514+
/*
515+
* Like sanitize_client, but we also perform any canonicalization
516+
* to make life easier on the admin.
517+
*/
518+
static char *canonicalize_client(const char *in)
519+
{
520+
struct strbuf out = STRBUF_INIT;
521+
sanitize_client_strbuf(&out, in);
522+
strbuf_tolower(&out);
523+
return strbuf_detach(&out, NULL);
524+
}
525+
487526
/*
488527
* Read the host as supplied by the client connection.
489528
*/
@@ -505,10 +544,10 @@ static void parse_host_arg(char *extra_args, int buflen)
505544
parse_host_and_port(val, &host, &port);
506545
if (port) {
507546
free(tcp_port);
508-
tcp_port = xstrdup(port);
547+
tcp_port = sanitize_client(port);
509548
}
510549
free(hostname);
511-
hostname = xstrdup_tolower(host);
550+
hostname = canonicalize_client(host);
512551
}
513552

514553
/* On to the next one */
@@ -541,8 +580,9 @@ static void parse_host_arg(char *extra_args, int buflen)
541580
ip_address = xstrdup(addrbuf);
542581

543582
free(canon_hostname);
544-
canon_hostname = xstrdup(ai->ai_canonname ?
545-
ai->ai_canonname : ip_address);
583+
canon_hostname = ai->ai_canonname ?
584+
sanitize_client(ai->ai_canonname) :
585+
xstrdup(ip_address);
546586

547587
freeaddrinfo(ai);
548588
}
@@ -564,7 +604,7 @@ static void parse_host_arg(char *extra_args, int buflen)
564604
addrbuf, sizeof(addrbuf));
565605

566606
free(canon_hostname);
567-
canon_hostname = xstrdup(hent->h_name);
607+
canon_hostname = sanitize_client(hent->h_name);
568608
free(ip_address);
569609
ip_address = xstrdup(addrbuf);
570610
}

t/t5570-git-daemon.sh

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,5 +141,32 @@ test_expect_success 'push disabled' "test_remote_error 'service not enab
141141
test_expect_success 'read access denied' "test_remote_error -x 'no such repository' fetch repo.git "
142142
test_expect_success 'not exported' "test_remote_error -n 'repository not exported' fetch repo.git "
143143

144+
stop_git_daemon
145+
start_git_daemon --interpolated-path="$GIT_DAEMON_DOCUMENT_ROOT_PATH/%H%D"
146+
147+
test_expect_success 'access repo via interpolated hostname' '
148+
repo="$GIT_DAEMON_DOCUMENT_ROOT_PATH/localhost/interp.git" &&
149+
git init --bare "$repo" &&
150+
git push "$repo" HEAD &&
151+
>"$repo"/git-daemon-export-ok &&
152+
rm -rf tmp.git &&
153+
GIT_OVERRIDE_VIRTUAL_HOST=localhost \
154+
git clone --bare "$GIT_DAEMON_URL/interp.git" tmp.git &&
155+
rm -rf tmp.git &&
156+
GIT_OVERRIDE_VIRTUAL_HOST=LOCALHOST \
157+
git clone --bare "$GIT_DAEMON_URL/interp.git" tmp.git
158+
'
159+
160+
test_expect_success 'hostname cannot break out of directory' '
161+
rm -rf tmp.git &&
162+
repo="$GIT_DAEMON_DOCUMENT_ROOT_PATH/../escape.git" &&
163+
git init --bare "$repo" &&
164+
git push "$repo" HEAD &&
165+
>"$repo"/git-daemon-export-ok &&
166+
test_must_fail \
167+
env GIT_OVERRIDE_VIRTUAL_HOST=.. \
168+
git clone --bare "$GIT_DAEMON_URL/escape.git" tmp.git
169+
'
170+
144171
stop_git_daemon
145172
test_done

0 commit comments

Comments
 (0)