Merge branch 'jk/daemon-interpolate'
authorJunio C Hamano <gitster@pobox.com>
Tue, 3 Mar 2015 22:37:05 +0000 (14:37 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 3 Mar 2015 22:37:06 +0000 (14:37 -0800)
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 062e133aa39b9e522bf3a03c5e57ac67c03ad88e,661af292db8bdb4b51d20fde9b6c70b23ec44d2d..372ac5ad9d16b7bc3f7e03012656b1541af5ba4c
+++ 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;
@@@ -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
                             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 09fa652fd172cf7bb5b1cf8a4f0ccbc072ca2111,b0b2b5382050edfc7fd7fc2b48e135e02b7fe582..c3edd960ec5bf686b66fa55dcfea712e455fe772
+++ 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];
        }
        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);
                }
  
                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;
        *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);
  
        }
  
        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 */
                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;
                        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);
                }
  #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 };