From: Junio C Hamano Date: Tue, 3 Mar 2015 22:37:05 +0000 (-0800) Subject: Merge branch 'jk/daemon-interpolate' X-Git-Tag: v2.4.0-rc0~75 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/4c3dbbf7225c45ab0e04953090d0ece4a15e83a2?ds=inline;hp=-c Merge branch 'jk/daemon-interpolate' 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 --- 4c3dbbf7225c45ab0e04953090d0ece4a15e83a2 diff --combined connect.c index 062e133aa3,661af292db..372ac5ad9d --- a/connect.c +++ b/connect.c @@@ -13,24 -13,28 +13,24 @@@ static char *server_capabilities; static const char *parse_feature_value(const char *, const char *, int *); -static int check_ref(const char *name, int len, unsigned int flags) +static int check_ref(const char *name, unsigned int flags) { if (!flags) return 1; - if (len < 5 || memcmp(name, "refs/", 5)) + if (!skip_prefix(name, "refs/", &name)) return 0; - /* Skip the "refs/" part */ - name += 5; - len -= 5; - /* REF_NORMAL means that we don't want the magic fake tag refs */ if ((flags & REF_NORMAL) && check_refname_format(name, 0)) return 0; /* REF_HEADS means that we want regular branch heads */ - if ((flags & REF_HEADS) && !memcmp(name, "heads/", 6)) + if ((flags & REF_HEADS) && starts_with(name, "heads/")) return 1; /* REF_TAGS means that we want tags */ - if ((flags & REF_TAGS) && !memcmp(name, "tags/", 5)) + if ((flags & REF_TAGS) && starts_with(name, "tags/")) return 1; /* All type bits clear means that we are ok with anything */ @@@ -39,7 -43,7 +39,7 @@@ int check_ref_type(const struct ref *ref, int flags) { - return check_ref(ref->name, strlen(ref->name), flags); + return check_ref(ref->name, flags); } static void die_initial_contact(int got_at_least_one_head) @@@ -93,7 -97,7 +93,7 @@@ static void annotate_refs_with_symref_i parse_one_symref_info(&symref, val, len); feature_list = val + 1; } - sort_string_list(&symref); + string_list_sort(&symref); for (; ref; ref = ref->next) { struct string_list_item *item; @@@ -163,7 -167,7 +163,7 @@@ struct ref **get_remote_heads(int in, c continue; } - if (!check_ref(name, name_len, flags)) + if (!check_ref(name, flags)) continue; ref = alloc_ref(buffer + 41); hashcpy(ref->old_sha1, old_sha1); @@@ -533,8 -537,7 +533,8 @@@ static struct child_process *git_proxy_ get_host_and_port(&host, &port); - proxy = xcalloc(1, sizeof(*proxy)); + proxy = xmalloc(sizeof(*proxy)); + child_process_init(proxy); argv_array_push(&proxy->args, git_proxy_command); argv_array_push(&proxy->args, host); argv_array_push(&proxy->args, port); @@@ -636,7 -639,7 +636,7 @@@ static enum protocol parse_connect_url( return protocol; } -static struct child_process no_fork; +static struct child_process no_fork = CHILD_PROCESS_INIT; /* * This returns a dummy child_process if the transport protocol does not @@@ -670,10 -673,20 +670,20 @@@ struct child_process *git_connect(int f printf("Diag: path=%s\n", path ? path : "NULL"); conn = NULL; } else if (protocol == PROTO_GIT) { + /* + * Set up virtual host information based on where we will + * connect, unless the user has overridden us in + * the environment. + */ + char *target_host = getenv("GIT_OVERRIDE_VIRTUAL_HOST"); + if (target_host) + target_host = xstrdup(target_host); + else + target_host = xstrdup(hostandport); + /* These underlying connection commands die() if they * cannot connect. */ - char *target_host = xstrdup(hostandport); if (git_use_proxy(hostandport)) conn = git_proxy_connect(fd, hostandport); else @@@ -691,8 -704,7 +701,8 @@@ target_host, 0); free(target_host); } else { - conn = xcalloc(1, sizeof(*conn)); + conn = xmalloc(sizeof(*conn)); + child_process_init(conn); strbuf_addstr(&cmd, prog); strbuf_addch(&cmd, ' '); @@@ -700,23 -712,14 +710,23 @@@ conn->in = conn->out = -1; if (protocol == PROTO_SSH) { - const char *ssh = getenv("GIT_SSH"); - int putty = ssh && strcasestr(ssh, "plink"); + const char *ssh; + int putty; char *ssh_host = hostandport; const char *port = NULL; get_host_and_port(&ssh_host, &port); port = get_port_numeric(port); - if (!ssh) ssh = "ssh"; + ssh = getenv("GIT_SSH_COMMAND"); + if (ssh) { + conn->use_shell = 1; + putty = 0; + } else { + ssh = getenv("GIT_SSH"); + if (!ssh) + ssh = "ssh"; + putty = !!strcasestr(ssh, "plink"); + } argv_array_push(&conn->args, ssh); if (putty && !strcasestr(ssh, "tortoiseplink")) diff --combined daemon.c index 09fa652fd1,b0b2b53820..c3edd960ec --- a/daemon.c +++ b/daemon.c @@@ -61,22 -61,6 +61,22 @@@ static char *canon_hostname static char *ip_address; static char *tcp_port; +static int hostname_lookup_done; + +static void lookup_hostname(void); + +static const char *get_canon_hostname(void) +{ + lookup_hostname(); + return canon_hostname; +} + +static const char *get_ip_address(void) +{ + lookup_hostname(); + return ip_address; +} + static void logreport(int priority, const char *err, va_list params) { if (log_syslog) { @@@ -122,46 -106,6 +122,46 @@@ static void NORETURN daemon_die(const c exit(1); } +static void strbuf_addstr_or_null(struct strbuf *sb, const char *s) +{ + if (s) + strbuf_addstr(sb, s); +} + +struct expand_path_context { + const char *directory; +}; + +static size_t expand_path(struct strbuf *sb, const char *placeholder, void *ctx) +{ + struct expand_path_context *context = ctx; + + switch (placeholder[0]) { + case 'H': + strbuf_addstr_or_null(sb, hostname); + return 1; + case 'C': + if (placeholder[1] == 'H') { + strbuf_addstr_or_null(sb, get_canon_hostname()); + return 2; + } + break; + case 'I': + if (placeholder[1] == 'P') { + strbuf_addstr_or_null(sb, get_ip_address()); + return 2; + } + break; + case 'P': + strbuf_addstr_or_null(sb, tcp_port); + return 1; + case 'D': + strbuf_addstr(sb, context->directory); + return 1; + } + return 0; +} + static const char *path_ok(const char *directory) { static char rpath[PATH_MAX]; @@@ -200,10 -144,14 +200,10 @@@ } else if (interpolated_path && saw_extended_args) { struct strbuf expanded_path = STRBUF_INIT; - struct strbuf_expand_dict_entry dict[6]; - - dict[0].placeholder = "H"; dict[0].value = hostname; - dict[1].placeholder = "CH"; dict[1].value = canon_hostname; - dict[2].placeholder = "IP"; dict[2].value = ip_address; - dict[3].placeholder = "P"; dict[3].value = tcp_port; - dict[4].placeholder = "D"; dict[4].value = directory; - dict[5].placeholder = NULL; dict[5].value = NULL; + struct expand_path_context context; + + context.directory = directory; + if (*dir != '/') { /* Allow only absolute */ logerror("'%s': Non-absolute path denied (interpolated-path active)", dir); @@@ -211,7 -159,7 +211,7 @@@ } strbuf_expand(&expanded_path, interpolated_path, - strbuf_expand_dict_cb, &dict); + expand_path, &context); strlcpy(interp_path, expanded_path.buf, PATH_MAX); strbuf_release(&expanded_path); loginfo("Interpolated dir '%s'", interp_path); @@@ -282,6 -230,23 +282,6 @@@ struct daemon_service int overridable; }; -static struct daemon_service *service_looking_at; -static int service_enabled; - -static int git_daemon_config(const char *var, const char *value, void *cb) -{ - const char *service; - - if (skip_prefix(var, "daemon.", &service) && - !strcmp(service, service_looking_at->config_name)) { - service_enabled = git_config_bool(var, value); - return 0; - } - - /* we are not interested in parsing any other configuration here */ - return 0; -} - static int daemon_error(const char *dir, const char *msg) { if (!informative_errors) @@@ -294,7 -259,7 +294,7 @@@ static const char *access_hook static int run_access_hook(struct daemon_service *service, const char *dir, const char *path) { - struct child_process child; + struct child_process child = CHILD_PROCESS_INIT; struct strbuf buf = STRBUF_INIT; const char *argv[8]; const char **arg = argv; @@@ -306,12 -271,13 +306,12 @@@ *arg++ = service->name; *arg++ = path; *arg++ = STRARG(hostname); - *arg++ = STRARG(canon_hostname); - *arg++ = STRARG(ip_address); + *arg++ = STRARG(get_canon_hostname()); + *arg++ = STRARG(get_ip_address()); *arg++ = STRARG(tcp_port); *arg = NULL; #undef STRARG - memset(&child, 0, sizeof(child)); child.use_shell = 1; child.argv = argv; child.no_stdin = 1; @@@ -358,7 -324,6 +358,7 @@@ static int run_service(const char *dir { const char *path; int enabled = service->enabled; + struct strbuf var = STRBUF_INIT; loginfo("Request %s for '%s'", service->name, dir); @@@ -389,9 -354,11 +389,9 @@@ } if (service->overridable) { - service_looking_at = service; - service_enabled = -1; - git_config(git_daemon_config, NULL); - if (0 <= service_enabled) - enabled = service_enabled; + strbuf_addf(&var, "daemon.%s", service->config_name); + git_config_get_bool(var.buf, &enabled); + strbuf_release(&var); } if (!enabled) { logerror("'%s': service not enabled for '%s'", @@@ -439,8 -406,9 +439,8 @@@ static void copy_to_log(int fd static int run_service_command(const char **argv) { - struct child_process cld; + struct child_process cld = CHILD_PROCESS_INIT; - memset(&cld, 0, sizeof(cld)); cld.argv = argv; cld.git_cmd = 1; cld.err = -1; @@@ -536,6 -504,45 +536,45 @@@ static void parse_host_and_port(char *h } } + /* + * Sanitize a string from the client so that it's OK to be inserted into a + * filesystem path. Specifically, we disallow slashes, runs of "..", and + * trailing and leading dots, which means that the client cannot escape + * our base path via ".." traversal. + */ + static void sanitize_client_strbuf(struct strbuf *out, const char *in) + { + for (; *in; in++) { + if (*in == '/') + continue; + if (*in == '.' && (!out->len || out->buf[out->len - 1] == '.')) + continue; + strbuf_addch(out, *in); + } + + while (out->len && out->buf[out->len - 1] == '.') + strbuf_setlen(out, out->len - 1); + } + + static char *sanitize_client(const char *in) + { + struct strbuf out = STRBUF_INIT; + sanitize_client_strbuf(&out, in); + return strbuf_detach(&out, NULL); + } + + /* + * Like sanitize_client, but we also perform any canonicalization + * to make life easier on the admin. + */ + static char *canonicalize_client(const char *in) + { + struct strbuf out = STRBUF_INIT; + sanitize_client_strbuf(&out, in); + strbuf_tolower(&out); + return strbuf_detach(&out, NULL); + } + /* * Read the host as supplied by the client connection. */ @@@ -557,11 -564,10 +596,11 @@@ static void parse_host_arg(char *extra_ parse_host_and_port(val, &host, &port); if (port) { free(tcp_port); - tcp_port = xstrdup(port); + tcp_port = sanitize_client(port); } free(hostname); - hostname = xstrdup_tolower(host); + hostname = canonicalize_client(host); + hostname_lookup_done = 0; } /* On to the next one */ @@@ -570,14 -576,11 +609,14 @@@ if (extra_args < end && *extra_args) die("Invalid request"); } +} - /* - * Locate canonical hostname and its IP address. - */ - if (hostname) { +/* + * Locate canonical hostname and its IP address. + */ +static void lookup_hostname(void) +{ + if (!hostname_lookup_done && hostname) { #ifndef NO_IPV6 struct addrinfo hints; struct addrinfo *ai; @@@ -597,8 -600,9 +636,9 @@@ ip_address = xstrdup(addrbuf); free(canon_hostname); - canon_hostname = xstrdup(ai->ai_canonname ? - ai->ai_canonname : ip_address); + canon_hostname = ai->ai_canonname ? + sanitize_client(ai->ai_canonname) : + xstrdup(ip_address); freeaddrinfo(ai); } @@@ -620,12 -624,11 +660,12 @@@ addrbuf, sizeof(addrbuf)); free(canon_hostname); - canon_hostname = xstrdup(hent->h_name); + canon_hostname = sanitize_client(hent->h_name); free(ip_address); ip_address = xstrdup(addrbuf); } #endif + hostname_lookup_done = 1; } } @@@ -771,7 -774,7 +811,7 @@@ static void check_dead_children(void static char **cld_argv; static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen) { - struct child_process cld = { NULL }; + struct child_process cld = CHILD_PROCESS_INIT; char addrbuf[300] = "REMOTE_ADDR=", portbuf[300]; char *env[] = { addrbuf, portbuf, NULL };