Canonicalize the request path once without the docroot and prepend the

docroot only only when it's needed.  Suggested by deraadt@.

diff --git a/usr.sbin/httpd/httpd.c b/usr.sbin/httpd/httpd.c
index 22ae5f9..454814d 100644 (file)
--- a/usr.sbin/httpd/httpd.c
+++ b/usr.sbin/httpd/httpd.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: httpd.c,v 1.8 2014/07/25 16:23:19 reyk Exp $  */
+/*     $OpenBSD: httpd.c,v 1.9 2014/07/25 21:29:58 reyk Exp $  */
 
 /*
  * Copyright (c) 2014 Reyk Floeter <reyk@openbsd.org>
@@ -466,24 +466,15 @@ canonicalize_host(const char *host, char *name, size_t len)
 }
 
 const char *
-canonicalize_path(const char *root, const char *input, char *path, size_t len)
+canonicalize_path(const char *input, char *path, size_t len)
 {
        const char      *i;
        char            *p, *start, *end;
-       size_t           n;
 
        /* assuming input starts with '/' and is nul-terminated */
        i = input;
        p = path;
 
-       /* prepend root directory, if specified */
-       if (root != NULL) {
-               if ((n = strlcpy(path, root, len)) >= len)
-                       return (NULL);
-               len -= n;
-               p += n;
-       }
-
        if (*input != '/' || len < 3)
                return (NULL);
 

diff --git a/usr.sbin/httpd/httpd.h b/usr.sbin/httpd/httpd.h
index cc10bfb..f2e3dce 100644 (file)
--- a/usr.sbin/httpd/httpd.h
+++ b/usr.sbin/httpd/httpd.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: httpd.h,v 1.12 2014/07/25 16:23:19 reyk Exp $ */
+/*     $OpenBSD: httpd.h,v 1.13 2014/07/25 21:29:58 reyk Exp $ */
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -421,7 +421,7 @@ void                 event_again(struct event *, int, short,
                    void (*)(int, short, void *),
                    struct timeval *, struct timeval *, void *);
 const char     *canonicalize_host(const char *, char *, size_t);
-const char     *canonicalize_path(const char *, const char *, char *, size_t);
+const char     *canonicalize_path(const char *, char *, size_t);
 void            imsg_event_add(struct imsgev *);
 int             imsg_compose_event(struct imsgev *, u_int16_t, u_int32_t,
                    pid_t, int, void *, u_int16_t);

diff --git a/usr.sbin/httpd/server_file.c b/usr.sbin/httpd/server_file.c
index 5a36029..81346ce 100644 (file)
--- a/usr.sbin/httpd/server_file.c
+++ b/usr.sbin/httpd/server_file.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: server_file.c,v 1.14 2014/07/25 20:13:06 reyk Exp $   */
+/*     $OpenBSD: server_file.c,v 1.15 2014/07/25 21:29:58 reyk Exp $   */
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -54,7 +54,9 @@ int
 server_file_access(struct http_descriptor *desc, char *path, size_t len,
     struct stat *st)
 {
+       char    *newpath;
        errno = 0;
+
        if (access(path, R_OK) == -1) {
                goto fail;
        } else if (stat(path, st) == -1) {
@@ -70,13 +72,10 @@ server_file_access(struct http_descriptor *desc, char *path, size_t len,
 
                /* Redirect to path with trailing "/" */
                if (path[strlen(path) - 1] != '/') {
-                       /* Remove the document root to get the relative URL */
-                       if (canonicalize_path(NULL,
-                           desc->http_path, path, len) == NULL ||
-                           strlcat(path, "/", len) >= len) {
-                               errno = EINVAL;
-                               goto fail;
-                       }
+                       if (asprintf(&newpath, "%s/", desc->http_path) == -1)
+                               return (500);
+                       free(desc->http_path);
+                       desc->http_path = newpath;
 
                        /* Indicate that the file has been moved */
                        return (301);
@@ -89,8 +88,7 @@ server_file_access(struct http_descriptor *desc, char *path, size_t len,
                }
 
                /* Check again but set len to 0 to avoid recursion */
-               if (server_file_access(desc, path, 0, st) != 0)
-                       goto fail;
+               return (server_file_access(desc, path, 0, st));
        } else if (!S_ISREG(st->st_mode)) {
                /* Don't follow symlinks and ignore special files */
                errno = EACCES;
@@ -100,10 +98,6 @@ server_file_access(struct http_descriptor *desc, char *path, size_t len,
        return (0);
 
  fail:
-       /* Remove the document root */
-       if (len && canonicalize_path(NULL, desc->http_path, path, len) == NULL)
-               return (500);
-
        switch (errno) {
        case ENOENT:
                return (404);
@@ -127,16 +121,17 @@ server_file(struct httpd *env, struct client *clt)
        char                     path[MAXPATHLEN];
        struct stat              st;
 
-       if (canonicalize_path(srv_conf->docroot,
-           desc->http_path, path, sizeof(path)) == NULL) {
+       /* Request path is already canonicalized */
+       if ((size_t)snprintf(path, sizeof(path), "%s/%s",
+           srv_conf->docroot, desc->http_path) >= sizeof(path)) {
                /* Do not echo the uncanonicalized path */
-               server_abort_http(clt, 500, "invalid request path");
+               server_abort_http(clt, 500, desc->http_path);
                return (-1);
        }
 
        /* Returns HTTP status code on error */
        if ((ret = server_file_access(desc, path, sizeof(path), &st)) != 0) {
-               server_abort_http(clt, ret, path);
+               server_abort_http(clt, ret, desc->http_path);
                return (-1);
        }
 

diff --git a/usr.sbin/httpd/server_http.c b/usr.sbin/httpd/server_http.c
index 98edb96..97d3a59 100644 (file)
--- a/usr.sbin/httpd/server_http.c
+++ b/usr.sbin/httpd/server_http.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: server_http.c,v 1.14 2014/07/25 16:23:19 reyk Exp $   */
+/*     $OpenBSD: server_http.c,v 1.15 2014/07/25 21:29:58 reyk Exp $   */
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -627,13 +627,19 @@ server_close_http(struct client *clt)
 int
 server_response(struct httpd *httpd, struct client *clt)
 {
+       char                     path[MAXPATHLEN];
        struct http_descriptor  *desc   = clt->clt_desc;
        struct server           *srv = clt->clt_srv;
        struct server_config    *srv_conf;
        struct kv               *kv, key;
        int                      ret;
 
-       if (desc->http_path == NULL)
+       /* Canonicalize the request path */
+       if (desc->http_path == NULL ||
+           canonicalize_path(desc->http_path, path, sizeof(path)) == NULL)
+               goto fail;
+       free(desc->http_path);
+       if ((desc->http_path = strdup(path)) == NULL)
                goto fail;
 
        if (strcmp(desc->http_version, "HTTP/1.1") == 0) {