struct stat from early file inspection was being used after actual file

open() which means the stat could refer to the wrong file.  Mostly this
relates to st_size use.  This bug could mean that httpd sends new files
truncated to the old length, saying "I am sure you have the correct file now"?
Could have other bad effects.
ok tb millert bluhm

1 files changed, 52 insertions, 49 deletions

diff --git a/usr.sbin/httpd/server_file.c b/usr.sbin/httpd/server_file.c
index 0027056db0d..5a0e778dff9 100644
--- a/usr.sbin/httpd/server_file.c
+++ b/usr.sbin/httpd/server_file.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server_file.c,v 1.72 2022/03/02 19:52:19 tb Exp $	*/
+/*	$OpenBSD: server_file.c,v 1.73 2022/03/02 23:27:43 deraadt Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2017 Reyk Floeter <reyk@openbsd.org>
@@ -40,13 +40,11 @@
 int		 server_file_access(struct httpd *, struct client *,
 		    char *, size_t);
 int		 server_file_request(struct httpd *, struct client *,
-		    char *, struct stat *);
+		    char *, time_t);
 int		 server_partial_file_request(struct httpd *, struct client *,
-		    char *, struct stat *, char *);
-int		 server_file_index(struct httpd *, struct client *,
-		    struct stat *);
-int		 server_file_modified_since(struct http_descriptor *,
-		    struct stat *);
+		    char *, time_t, char *);
+int		 server_file_index(struct httpd *, struct client *);
+int		 server_file_modified_since(struct http_descriptor *, time_t);
 int		 server_file_method(struct client *);
 int		 parse_range_spec(char *, size_t, struct range *);
 int		 parse_ranges(struct client *, char *, size_t);
@@ -64,9 +62,7 @@ server_file_access(struct httpd *env, struct client *clt,
 
 	errno = 0;
 
-	if (access(path, R_OK) == -1) {
-		goto fail;
-	} else if (stat(path, &st) == -1) {
+	if (stat(path, &st) == -1) {
 		goto fail;
 	} else if (S_ISDIR(st.st_mode)) {
 		/* Deny access if directory indexing is disabled */
@@ -127,7 +123,7 @@ server_file_access(struct httpd *env, struct client *clt,
 				goto fail;
 			}
 
-			return (server_file_index(env, clt, &st));
+			return (server_file_index(env, clt));
 		}
 		return (ret);
 	} else if (!S_ISREG(st.st_mode)) {
@@ -139,10 +135,10 @@ server_file_access(struct httpd *env, struct client *clt,
 	key.kv_key = "Range";
 	r = kv_find(&desc->http_headers, &key);
 	if (r != NULL)
-		return (server_partial_file_request(env, clt, path, &st,
-		    r->kv_value));
+		return (server_partial_file_request(env, clt, path,
+		    st.st_mtim.tv_sec, r->kv_value));
 	else
-		return (server_file_request(env, clt, path, &st));
+		return (server_file_request(env, clt, path, st.st_mtim.tv_sec));
 
  fail:
 	switch (errno) {
@@ -216,14 +212,14 @@ server_file_method(struct client *clt)
 
 int
 server_file_request(struct httpd *env, struct client *clt, char *path,
-    struct stat *st)
+    time_t mtim)
 {
 	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;
 	size_t			 bufsiz;
-	struct stat		 gzst;
 	char			 gzpath[PATH_MAX];
 
 	if ((ret = server_file_method(clt)) != 0) {
@@ -232,11 +228,10 @@ server_file_request(struct httpd *env, struct client *clt, char *path,
 	}
 
 	media = media_find_config(env, srv_conf, path);
-
-	if ((ret = server_file_modified_since(clt->clt_descreq, st)) != -1) {
+	if ((ret = server_file_modified_since(clt->clt_descreq, mtim)) != -1) {
 		/* send the header without a body */
 		if ((ret = server_response_http(clt, ret, media, -1,
-		    MINIMUM(time(NULL), st->st_mtim.tv_sec))) == -1)
+		    MINIMUM(time(NULL), mtim))) == -1)
 			goto fail;
 		goto done;
 	}
@@ -256,22 +251,22 @@ server_file_request(struct httpd *env, struct client *clt, char *path,
 			ret = snprintf(gzpath, sizeof(gzpath), "%s.gz", path);
 			if (ret < 0 || (size_t)ret >= sizeof(gzpath))
 				goto abort;
-			if (access(gzpath, R_OK) == 0 &&
-			    stat(gzpath, &gzst) == 0) {
-				path = gzpath;
-				st = &gzst;
+			if ((fd = open(gzpath, O_RDONLY)) > 0)
 				kv_add(&resp->http_headers,
 				    "Content-Encoding", "gzip");
-			}
 		}
 	}
 
 	/* Now open the file, should be readable or we have another problem */
-	if ((fd = open(path, O_RDONLY)) == -1)
+	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;
@@ -296,7 +291,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);
 
@@ -323,7 +318,7 @@ 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 stat *st, char *range_str)
+    time_t mtim, char *range_str)
 {
 	struct server_config	*srv_conf = clt->clt_srv_conf;
 	struct http_descriptor	*resp = clt->clt_descresp;
@@ -332,26 +327,29 @@ server_partial_file_request(struct httpd *env, struct client *clt, char *path,
 	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;
 	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, st);
+		return server_file_request(env, clt, path, mtim);
 
-	if ((nranges = parse_ranges(clt, range_str, st->st_size)) < 1) {
+	/* 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) {
 		code = 416;
 		(void)snprintf(content_range, sizeof(content_range),
-		    "bytes */%lld", st->st_size);
+		    "bytes */%lld", st.st_size);
 		errstr = content_range;
 		goto abort;
 	}
 
-	/* Now open the file, should be readable or we have another problem */
-	if ((fd = open(path, O_RDONLY)) == -1)
-		goto abort;
-
 	media = media_find_config(env, srv_conf, path);
 	r->range_media = media;
 
@@ -359,7 +357,7 @@ server_partial_file_request(struct httpd *env, struct client *clt, char *path,
 		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;
@@ -381,7 +379,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 */
@@ -406,7 +404,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;
@@ -431,7 +429,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);
 
@@ -457,7 +455,7 @@ server_partial_file_request(struct httpd *env, struct client *clt, char *path,
 }
 
 int
-server_file_index(struct httpd *env, struct client *clt, struct stat *st)
+server_file_index(struct httpd *env, struct client *clt)
 {
 	char			  path[PATH_MAX];
 	char			  tmstr[21];
@@ -471,6 +469,7 @@ server_file_index(struct httpd *env, struct client *clt, struct stat *st)
 	const char		 *stripped, *style;
 	char			 *escapeduri, *escapedhtml, *escapedpath;
 	struct tm		  tm;
+	struct stat		  st;
 	time_t			  t, dir_mtime;
 
 	if ((ret = server_file_method(clt)) != 0) {
@@ -487,9 +486,11 @@ server_file_index(struct httpd *env, struct client *clt, struct stat *st)
 	/* 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;
@@ -528,15 +529,17 @@ server_file_index(struct httpd *env, struct client *clt, struct stat *st)
 	free(escapedpath);
 
 	for (i = 0; i < namesize; i++) {
+		struct stat subst;
+
 		dp = namelist[i];
 
 		if (skip ||
-		    fstatat(fd, dp->d_name, st, 0) == -1) {
+		    fstatat(fd, dp->d_name, &subst, 0) == -1) {
 			free(dp);
 			continue;
 		}
 
-		t = st->st_mtime;
+		t = subst.st_mtime;
 		localtime_r(&t, &tm);
 		strftime(tmstr, sizeof(tmstr), "%d-%h-%Y %R", &tm);
 		namewidth = 51 - strlen(dp->d_name);
@@ -549,7 +552,7 @@ server_file_index(struct httpd *env, struct client *clt, struct stat *st)
 		if (dp->d_name[0] == '.' &&
 		    !(dp->d_name[1] == '.' && dp->d_name[2] == '\0')) {
 			/* ignore hidden files starting with a dot */
-		} else if (S_ISDIR(st->st_mode)) {
+		} else if (S_ISDIR(subst.st_mode)) {
 			namewidth -= 1; /* trailing slash */
 			if (evbuffer_add_printf(evb,
 			    "<a href=\"%s%s/\">%s/</a>%*s%s%20s\n",
@@ -557,13 +560,13 @@ server_file_index(struct httpd *env, struct client *clt, struct stat *st)
 			    escapeduri, escapedhtml,
 			    MAXIMUM(namewidth, 0), " ", tmstr, "-") == -1)
 				skip = 1;
-		} else if (S_ISREG(st->st_mode)) {
+		} else if (S_ISREG(subst.st_mode)) {
 			if (evbuffer_add_printf(evb,
 			    "<a href=\"%s%s\">%s</a>%*s%s%20llu\n",
 			    strchr(escapeduri, ':') != NULL ? "./" : "",
 			    escapeduri, escapedhtml,
 			    MAXIMUM(namewidth, 0), " ",
-			    tmstr, st->st_size) == -1)
+			    tmstr, subst.st_size) == -1)
 				skip = 1;
 		}
 		free(escapeduri);
@@ -683,7 +686,7 @@ server_file_error(struct bufferevent *bev, short error, void *arg)
 }
 
 int
-server_file_modified_since(struct http_descriptor *desc, struct stat *st)
+server_file_modified_since(struct http_descriptor *desc, time_t mtim)
 {
 	struct kv	 key, *since;
 	struct tm	 tm;
@@ -699,7 +702,7 @@ server_file_modified_since(struct http_descriptor *desc, struct stat *st)
 		 */
 		if (strptime(since->kv_value,
 		    "%a, %d %h %Y %T %Z", &tm) != NULL &&
-		    timegm(&tm) >= st->st_mtim.tv_sec)
+		    timegm(&tm) >= mtim)
 			return (304);
 	}