daemon: sanitize incoming virtual hostname

We use the daemon_avoid_alias function to make sure that the pathname the user gives us is sane. However, after applying that check, we might then interpolate the path using a string given by the server admin, but which may contain more untrusted data from the client. We should be sure to sanitize this data, as well. We cannot use daemon_avoid_alias here, as it is more strict than we need in requiring a leading '/'. At the same time, we can be much more strict here. We are interpreting a hostname, which should not contain slashes or excessive runs of dots, as those things are not allowed in DNS names. Note that in addition to cleansing the hostname field, we must check the "canonical hostname" (%CH) as well as the port (%P), which we take as a raw string. For the canonical hostname, this comes from an actual DNS lookup on the accessed IP, which makes it a much less likely vector for problems. But it does not hurt to sanitize it in the same way. Unfortunately we cannot test this case easily, as it would involve a custom hostname lookup. We do not need to check %IP, as it comes straight from inet_ntop, so must have a sane form. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 17, 2015 at 14:09 UTC b48537305229d1a4f25633f71941ee52d2582017
2 files changed +56 -5
daemon.c
+45 -5
@@ -504,6 +504,45 @@ static void parse_host_and_port(char *hostport, char **host,
504 }
505 }
506
507 +/*
508 + * Sanitize a string from the client so that it's OK to be inserted into a
509 + * filesystem path. Specifically, we disallow slashes, runs of "..", and
510 + * trailing and leading dots, which means that the client cannot escape
511 + * our base path via ".." traversal.
512 + */
513 +static void sanitize_client_strbuf(struct strbuf *out, const char *in)
514 +{
515 + for (; *in; in++) {
516 + if (*in == '/')
517 + continue;
518 + if (*in == '.' && (!out->len || out->buf[out->len - 1] == '.'))
519 + continue;
520 + strbuf_addch(out, *in);
521 + }
522 +
523 + while (out->len && out->buf[out->len - 1] == '.')
524 + strbuf_setlen(out, out->len - 1);
525 +}
526 +
527 +static char *sanitize_client(const char *in)
528 +{
529 + struct strbuf out = STRBUF_INIT;
530 + sanitize_client_strbuf(&out, in);
531 + return strbuf_detach(&out, NULL);
532 +}
533 +
534 +/*
535 + * Like sanitize_client, but we also perform any canonicalization
536 + * to make life easier on the admin.
537 + */
538 +static char *canonicalize_client(const char *in)
539 +{
540 + struct strbuf out = STRBUF_INIT;
541 + sanitize_client_strbuf(&out, in);
542 + strbuf_tolower(&out);
543 + return strbuf_detach(&out, NULL);
544 +}
545 +
546 /*
547 * Read the host as supplied by the client connection.
548 */
@@ -525,10 +564,10 @@ static void parse_host_arg(char *extra_args, int buflen)
564 parse_host_and_port(val, &host, &port);
565 if (port) {
566 free(tcp_port);
528 - tcp_port = xstrdup(port);
567 + tcp_port = sanitize_client(port);
568 }
569 free(hostname);
531 - hostname = xstrdup_tolower(host);
570 + hostname = canonicalize_client(host);
571 }
572
573 /* On to the next one */
@@ -561,8 +600,9 @@ static void parse_host_arg(char *extra_args, int buflen)
600 ip_address = xstrdup(addrbuf);
601
602 free(canon_hostname);
564 - canon_hostname = xstrdup(ai->ai_canonname ?
565 - ai->ai_canonname : ip_address);
603 + canon_hostname = ai->ai_canonname ?
604 + sanitize_client(ai->ai_canonname) :
605 + xstrdup(ip_address);
606
607 freeaddrinfo(ai);
608 }
@@ -584,7 +624,7 @@ static void parse_host_arg(char *extra_args, int buflen)
624 addrbuf, sizeof(addrbuf));
625
626 free(canon_hostname);
587 - canon_hostname = xstrdup(hent->h_name);
627 + canon_hostname = sanitize_client(hent->h_name);
628 free(ip_address);
629 ip_address = xstrdup(addrbuf);
630 }
t/t5570-git-daemon.sh
+11
@@ -157,5 +157,16 @@ test_expect_success 'access repo via interpolated hostname' '
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 +
171 stop_git_daemon
172 test_done