Merge branch 'jk/daemon-interpolate' into maint
authorJunio C Hamano <gitster@pobox.com>
Sat, 14 Mar 2015 05:55:59 +0000 (22:55 -0700)
committerJunio C Hamano <gitster@pobox.com>
Sat, 14 Mar 2015 05:55:59 +0000 (22:55 -0700)
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

1  2 
connect.c
daemon.c
diff --combined connect.c
index 2a5c400494f59c974894d18144dfcce6271df139,661af292db8bdb4b51d20fde9b6c70b23ec44d2d..d50f52ad8b1b8526c2c591d15a1955df96d59f76
+++ b/connect.c
  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;
@@@ -157,12 -161,13 +157,12 @@@ struct ref **get_remote_heads(int in, c
                        server_capabilities = xstrdup(name + name_len + 1);
                }
  
 -              if (extra_have &&
 -                  name_len == 5 && !memcmp(".have", name, 5)) {
 +              if (extra_have && !strcmp(name, ".have")) {
                        sha1_array_append(extra_have, old_sha1);
                        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);
@@@ -532,8 -537,7 +532,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);
@@@ -635,7 -639,7 +635,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
@@@ -669,10 -673,20 +669,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
                             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, ' ');
  
                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 54a03bd527a81485b498e33225ed54c54c0dbff7,b0b2b5382050edfc7fd7fc2b48e135e02b7fe582..26d5f326e468ea051bf6006f1a69bc7a4b21eaf1
+++ b/daemon.c
@@@ -230,6 -230,23 +230,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)
@@@ -242,7 -259,7 +242,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;
        *arg = NULL;
  #undef STRARG
  
 -      memset(&child, 0, sizeof(child));
        child.use_shell = 1;
        child.argv = argv;
        child.no_stdin = 1;
@@@ -306,7 -324,6 +306,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);
  
        }
  
        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'",
@@@ -387,8 -406,9 +387,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;
@@@ -484,6 -504,45 +484,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.
   */
@@@ -505,10 -564,10 +544,10 @@@ 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);
                        }
  
                        /* On to the next one */
                        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);
                }
                                  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);
                }
@@@ -714,7 -774,7 +754,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 };