Prevent toctu issues in static file serving and auto index generation.
authorflorian <florian@openbsd.org>
Tue, 16 Apr 2024 17:15:50 +0000 (17:15 +0000)
committerflorian <florian@openbsd.org>
Tue, 16 Apr 2024 17:15:50 +0000 (17:15 +0000)
This fixes a problem in passing, reported by matthieu@ where httpd
would return 500 Internal Server Error when it could stat(2) but not
open(2) a file. The correct error code is 403.

testing matthieu
ok tobhe, tl;dr ok stsp
input & OK deraadt

usr.sbin/httpd/server_file.c

index 8e76cc9..851ad46 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: server_file.c,v 1.78 2024/01/06 11:29:00 espie Exp $  */
+/*     $OpenBSD: server_file.c,v 1.79 2024/04/16 17:15:50 florian Exp $        */
 
 /*
  * Copyright (c) 2006 - 2017 Reyk Floeter <reyk@openbsd.org>
 int             server_file_access(struct httpd *, struct client *,
                    char *, size_t);
 int             server_file_request(struct httpd *, struct client *,
-                   char *, struct timespec *);
+                   struct media_type *, int, const struct stat *);
 int             server_partial_file_request(struct httpd *, struct client *,
-                   char *, struct timespec *, char *);
-int             server_file_index(struct httpd *, struct client *);
+                   struct media_type *, int, const struct stat *,
+                   char *);
+int             server_file_index(struct httpd *, struct client *, int,
+                   struct stat *);
 int             server_file_modified_since(struct http_descriptor *,
-                   struct timespec *);
+                   const struct timespec *);
 int             server_file_method(struct client *);
 int             parse_range_spec(char *, size_t, struct range *);
 int             parse_ranges(struct client *, char *, size_t);
@@ -62,28 +64,42 @@ server_file_access(struct httpd *env, struct client *clt,
        struct server_config    *srv_conf = clt->clt_srv_conf;
        struct stat              st;
        struct kv               *r, key;
+       struct media_type       *media;
        char                    *newpath, *encodedpath;
-       int                      ret;
-
-       errno = 0;
+       int                      ret, fd;
+
+       if ((fd = open(path, O_RDONLY)) == -1) {
+               switch (errno) {
+               case ENOENT:
+               case ENOTDIR:
+                       return (404);
+               case EACCES:
+                       return (403);
+               default:
+                       return (500);
+               }
+       }
+       if (fstat(fd, &st) == -1) {
+               close(fd);
+               return (500);
+       }
 
-       if (stat(path, &st) == -1) {
-               goto fail;
-       } else if (S_ISDIR(st.st_mode)) {
+       if (S_ISDIR(st.st_mode)) {
                /* Deny access if directory indexing is disabled */
                if (srv_conf->flags & SRVFLAG_NO_INDEX) {
-                       errno = EACCES;
-                       goto fail;
+                       close(fd);
+                       return (403);
                }
 
                if (desc->http_path_alias != NULL) {
                        /* Recursion - the index "file" is a directory? */
-                       errno = EINVAL;
-                       goto fail;
+                       close(fd);
+                       return (500);
                }
 
                /* Redirect to path with trailing "/" */
                if (path[strlen(path) - 1] != '/') {
+                       close(fd);
                        if ((encodedpath = url_encode(desc->http_path)) == NULL)
                                return (500);
                        if (asprintf(&newpath, "%s/", encodedpath) == -1) {
@@ -101,18 +117,21 @@ server_file_access(struct httpd *env, struct client *clt,
 
                /* Append the default index file to the location */
                if (asprintf(&newpath, "%s%s", desc->http_path,
-                   srv_conf->index) == -1)
+                   srv_conf->index) == -1) {
+                       close(fd);
                        return (500);
+               }
                desc->http_path_alias = newpath;
                if (server_getlocation(clt, newpath) != srv_conf) {
                        /* The location has changed */
+                       close(fd);
                        return (server_file(env, clt));
                }
 
                /* Otherwise append the default index file to the path */
                if (strlcat(path, srv_conf->index, len) >= len) {
-                       errno = EACCES;
-                       goto fail;
+                       close(fd);
+                       return (403);
                }
 
                ret = server_file_access(env, clt, path, len);
@@ -124,39 +143,72 @@ server_file_access(struct httpd *env, struct client *clt,
                         * stat.
                         */
                        if ((srv_conf->flags & SRVFLAG_AUTO_INDEX) == 0) {
-                               errno = EACCES;
-                               goto fail;
+                               close(fd);
+                               return (403);
                        }
 
-                       return (server_file_index(env, clt));
+                       return (server_file_index(env, clt, fd, &st));
                }
+               close(fd);
                return (ret);
        } else if (!S_ISREG(st.st_mode)) {
                /* Don't follow symlinks and ignore special files */
-               errno = EACCES;
-               goto fail;
+               close(fd);
+               return (403);
        }
 
-       key.kv_key = "Range";
-       r = kv_find(&desc->http_headers, &key);
-       if (r != NULL)
-               return (server_partial_file_request(env, clt, path,
-                   &st.st_mtim, r->kv_value));
-       else
-               return (server_file_request(env, clt, path, &st.st_mtim));
+       media = media_find_config(env, srv_conf, path);
 
- fail:
-       switch (errno) {
-       case ENOENT:
-       case ENOTDIR:
-               return (404);
-       case EACCES:
-               return (403);
-       default:
-               return (500);
+       /* Only consider range requests for GET */
+       if (desc->http_method == HTTP_METHOD_GET) {
+               key.kv_key = "Range";
+               r = kv_find(&desc->http_headers, &key);
+               if (r != NULL)
+                       return (server_partial_file_request(env, clt, media,
+                           fd, &st, r->kv_value));
        }
 
-       /* NOTREACHED */
+       /* change path to path.gz if necessary. */
+       if (srv_conf->flags & SRVFLAG_GZIP_STATIC) {
+               struct http_descriptor  *req = clt->clt_descreq;
+               struct http_descriptor  *resp = clt->clt_descresp;
+               struct stat              gzst;
+               int                      gzfd;
+               char                     gzpath[PATH_MAX];
+
+               /* check Accept-Encoding header */
+               key.kv_key = "Accept-Encoding";
+               r = kv_find(&req->http_headers, &key);
+
+               if (r != NULL && strstr(r->kv_value, "gzip") != NULL) {
+                       /* append ".gz" to path and check existence */
+                       ret = snprintf(gzpath, sizeof(gzpath), "%s.gz", path);
+                       if (ret < 0 || (size_t)ret >= sizeof(gzpath)) {
+                               close(fd);
+                               return (500);
+                       }
+
+                       if ((gzfd = open(gzpath, O_RDONLY)) != -1) {
+                               /* .gz must be a file, and not older */
+                               if (fstat(gzfd, &gzst) != -1 &&
+                                   S_ISREG(gzst.st_mode) &&
+                                   timespeccmp(&gzst.st_mtim, &st.st_mtim,
+                                   >=)) {
+                                       kv_add(&resp->http_headers,
+                                           "Content-Encoding", "gzip");
+                                       /* Use original file timestamp */
+                                       gzst.st_mtim = st.st_mtim;
+                                       st = gzst;
+                                       close(fd);
+                                       fd = gzfd;
+                               } else {
+                                       close(gzfd);
+                               }
+                       }
+               }
+       }
+
+       return (server_file_request(env, clt, media, fd, &st));
 }
 
 int
@@ -216,73 +268,30 @@ server_file_method(struct client *clt)
 }
 
 int
-server_file_request(struct httpd *env, struct client *clt, char *path,
-    struct timespec *mtim)
+server_file_request(struct httpd *env, struct client *clt, struct media_type
+    *media, int fd, const struct stat *st)
 {
        struct server_config    *srv_conf = clt->clt_srv_conf;
-       struct media_type       *media;
        const char              *errstr = NULL;
-       int                      fd = -1, ret, code = 500;
-       struct stat              st;
+       int                      ret, code = 500;
        size_t                   bufsiz;
-       char                     gzpath[PATH_MAX];
 
        if ((ret = server_file_method(clt)) != 0) {
                code = ret;
                goto abort;
        }
 
-       media = media_find_config(env, srv_conf, path);
-       if ((ret = server_file_modified_since(clt->clt_descreq, mtim)) != -1) {
+       if ((ret = server_file_modified_since(clt->clt_descreq, &st->st_mtim))
+           != -1) {
                /* send the header without a body */
                if ((ret = server_response_http(clt, ret, media, -1,
-                   MINIMUM(time(NULL), mtim->tv_sec))) == -1)
+                   MINIMUM(time(NULL), st->st_mtim.tv_sec))) == -1)
                        goto fail;
                goto done;
        }
 
-       /* change path to path.gz if necessary. */
-       if (srv_conf->flags & SRVFLAG_GZIP_STATIC) {
-               struct http_descriptor  *req = clt->clt_descreq;
-               struct http_descriptor  *resp = clt->clt_descresp;
-               struct kv               *r, key;
-
-               /* check Accept-Encoding header */
-               key.kv_key = "Accept-Encoding";
-               r = kv_find(&req->http_headers, &key);
-
-               if (r != NULL && strstr(r->kv_value, "gzip") != NULL) {
-                       /* append ".gz" to path and check existence */
-                       ret = snprintf(gzpath, sizeof(gzpath), "%s.gz", path);
-                       if (ret < 0 || (size_t)ret >= sizeof(gzpath))
-                               goto abort;
-                       if ((fd = open(gzpath, O_RDONLY)) != -1) {
-                               /* .gz must be a file, and not older */
-                               if (fstat(fd, &st) != -1 &&
-                                   S_ISREG(st.st_mode) &&
-                                   timespeccmp(&st.st_mtim, mtim, >=)) {
-                                       kv_add(&resp->http_headers,
-                                           "Content-Encoding", "gzip");
-                                       /* Use original file timestamp */
-                                       st.st_mtim = *mtim;
-                               } else {
-                                       close(fd);
-                                       fd = -1;
-                               }
-                       }
-               }
-       }
-
-       /* Now open the file, should be readable or we have another problem */
-       if (fd == -1) {
-               if ((fd = open(path, O_RDONLY)) == -1)
-                       goto abort;
-               if (fstat(fd, &st) == -1)
-                       goto abort;
-       }
-
-       ret = server_response_http(clt, 200, media, st.st_size,
-           MINIMUM(time(NULL), st.st_mtim.tv_sec));
+       ret = server_response_http(clt, 200, media, st->st_size,
+           MINIMUM(time(NULL), st->st_mtim.tv_sec));
        switch (ret) {
        case -1:
                goto fail;
@@ -307,7 +316,7 @@ server_file_request(struct httpd *env, struct client *clt, char *path,
        }
 
        /* Adjust read watermark to the optimal file io size */
-       bufsiz = MAXIMUM(st.st_blksize, 64 * 1024);
+       bufsiz = MAXIMUM(st->st_blksize, 64 * 1024);
        bufferevent_setwatermark(clt->clt_srvbev, EV_READ, 0,
            bufsiz);
 
@@ -333,47 +342,34 @@ server_file_request(struct httpd *env, struct client *clt, char *path,
 }
 
 int
-server_partial_file_request(struct httpd *env, struct client *clt, char *path,
-    struct timespec *mtim, char *range_str)
+server_partial_file_request(struct httpd *env, struct client *clt,
+    struct media_type *media, int fd, const struct stat *st, char *range_str)
 {
        struct server_config    *srv_conf = clt->clt_srv_conf;
        struct http_descriptor  *resp = clt->clt_descresp;
-       struct http_descriptor  *desc = clt->clt_descreq;
-       struct media_type       *media, multipart_media;
+       struct media_type        multipart_media;
        struct range_data       *r = &clt->clt_ranges;
        struct range            *range;
        size_t                   content_length = 0, bufsiz;
-       struct stat              st;
-       int                      code = 500, fd = -1, i, nranges, ret;
+       int                      code = 500, i, nranges, ret;
        char                     content_range[64];
        const char              *errstr = NULL;
 
-       /* Ignore range request for methods other than GET */
-       if (desc->http_method != HTTP_METHOD_GET)
-               return server_file_request(env, clt, path, mtim);
-
-       /* Now open the file, should be readable or we have another problem */
-       if ((fd = open(path, O_RDONLY)) == -1)
-               goto abort;
-       if (fstat(fd, &st) == -1)
-               goto abort;
-
-       if ((nranges = parse_ranges(clt, range_str, st.st_size)) < 1) {
+       if ((nranges = parse_ranges(clt, range_str, st->st_size)) < 1) {
                code = 416;
                (void)snprintf(content_range, sizeof(content_range),
-                   "bytes */%lld", st.st_size);
+                   "bytes */%lld", st->st_size);
                errstr = content_range;
                goto abort;
        }
 
-       media = media_find_config(env, srv_conf, path);
        r->range_media = media;
 
        if (nranges == 1) {
                range = &r->range[0];
                (void)snprintf(content_range, sizeof(content_range),
                    "bytes %lld-%lld/%lld", range->start, range->end,
-                   st.st_size);
+                   st->st_size);
                if (kv_add(&resp->http_headers, "Content-Range",
                    content_range) == NULL)
                        goto abort;
@@ -395,7 +391,7 @@ server_partial_file_request(struct httpd *env, struct client *clt, char *path,
                            "Content-Range: bytes %lld-%lld/%lld\r\n\r\n",
                            clt->clt_boundary,
                            media->media_type, media->media_subtype,
-                           range->start, range->end, st.st_size)) < 0)
+                           range->start, range->end, st->st_size)) < 0)
                                goto abort;
 
                        /* Add data length */
@@ -420,7 +416,7 @@ server_partial_file_request(struct httpd *env, struct client *clt, char *path,
        r->range_toread = TOREAD_HTTP_RANGE;
 
        ret = server_response_http(clt, 206, media, content_length,
-           MINIMUM(time(NULL), st.st_mtim.tv_sec));
+           MINIMUM(time(NULL), st->st_mtim.tv_sec));
        switch (ret) {
        case -1:
                goto fail;
@@ -445,7 +441,7 @@ server_partial_file_request(struct httpd *env, struct client *clt, char *path,
        }
 
        /* Adjust read watermark to the optimal file io size */
-       bufsiz = MAXIMUM(st.st_blksize, 64 * 1024);
+       bufsiz = MAXIMUM(st->st_blksize, 64 * 1024);
        bufferevent_setwatermark(clt->clt_srvbev, EV_READ, 0,
            bufsiz);
 
@@ -482,21 +478,21 @@ select_visible(const struct dirent *dp)
 }
 
 int
-server_file_index(struct httpd *env, struct client *clt)
+server_file_index(struct httpd *env, struct client *clt, int fd,
+    struct stat *st)
 {
        char                      path[PATH_MAX];
        char                      tmstr[21];
        struct http_descriptor   *desc = clt->clt_descreq;
        struct server_config     *srv_conf = clt->clt_srv_conf;
        struct dirent           **namelist, *dp;
-       int                       namesize, i, ret, fd = -1, skip;
+       int                       namesize, i, ret, skip;
        int                       code = 500;
        struct evbuffer          *evb = NULL;
        struct media_type        *media;
        const char               *stripped;
        char                     *escapeduri, *escapedhtml, *escapedpath;
        struct tm                 tm;
-       struct stat               st;
        time_t                    t, dir_mtime;
        char                      human_size[FMT_SCALED_STRSIZE];
 
@@ -511,14 +507,8 @@ server_file_index(struct httpd *env, struct client *clt)
            srv_conf->root, stripped) >= sizeof(path))
                goto abort;
 
-       /* Now open the file, should be readable or we have another problem */
-       if ((fd = open(path, O_RDONLY)) == -1)
-               goto abort;
-       if (fstat(fd, &st) == -1)
-               goto abort;
-
        /* Save last modification time */
-       dir_mtime = MINIMUM(time(NULL), st.st_mtim.tv_sec);
+       dir_mtime = MINIMUM(time(NULL), st->st_mtim.tv_sec);
 
        if ((evb = evbuffer_new()) == NULL)
                goto abort;
@@ -548,7 +538,7 @@ server_file_index(struct httpd *env, struct client *clt)
 
        free(escapedpath);
 
-       if ((namesize = scandir(path, &namelist, select_visible, 
+       if ((namesize = scandirat(fd, ".", &namelist, select_visible,
            alphasort)) == -1)
                goto abort;
 
@@ -722,7 +712,8 @@ server_file_error(struct bufferevent *bev, short error, void *arg)
 }
 
 int
-server_file_modified_since(struct http_descriptor *desc, struct timespec *mtim)
+server_file_modified_since(struct http_descriptor *desc, const struct timespec
+    *mtim)
 {
        struct kv        key, *since;
        struct tm        tm;