I was looking for something different when I stumbled upon this.
- in server_file_index `escapedpath' is leaked if evbuffer_add_printf
fails, similarly the directory entries in the loop. `escapeduri'
is also leaked if escape_html fails.
- read_errdoc leaks a file descriptor if fstat fails or if the file
is empty.
both very unlikely but worth fixing IMHO.
For server_file_index I decided to just drop the `skip' variable and
just goto fail on errors. There shouldn't be any visible change from
it, except that we don't try to serve half page if we somehow exhaust
memory halfway through.
ok?
Index: server_file.c
===================================================================
RCS file: /home/cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.74
diff -u -p -r1.74 server_file.c
--- server_file.c 4 Mar 2022 01:46:07 -0000 1.74
+++ server_file.c 14 Aug 2022 21:09:25 -0000
@@ -474,7 +474,7 @@ server_file_index(struct httpd *env, str
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, namewidth, skip;
+ int namesize = 0, i = 0, ret, fd = -1, namewidth;
int code = 500;
struct evbuffer *evb = NULL;
struct media_type *media;
@@ -510,9 +510,6 @@ server_file_index(struct httpd *env, str
if ((namesize = scandir(path, &namelist, NULL, alphasort)) == -1)
goto abort;
- /* Indicate failure but continue going through the list */
- skip = 0;
-
if ((escapedpath = escape_html(desc->http_path)) == NULL)
goto fail;
@@ -535,8 +532,10 @@ server_file_index(struct httpd *env, str
"<body>\n"
"<h1>Index of %s</h1>\n"
"<hr>\n<pre>\n",
- escapedpath, style, escapedpath) == -1)
- skip = 1;
+ escapedpath, style, escapedpath) == -1) {
+ free(escapedpath);
+ goto fail;
+ }
free(escapedpath);
@@ -545,8 +544,7 @@ server_file_index(struct httpd *env, str
dp = namelist[i];
- if (skip ||
- fstatat(fd, dp->d_name, &subst, 0) == -1) {
+ if (fstatat(fd, dp->d_name, &subst, 0) == -1) {
free(dp);
continue;
}
@@ -558,8 +556,10 @@ server_file_index(struct httpd *env, str
if ((escapeduri = url_encode(dp->d_name)) == NULL)
goto fail;
- if ((escapedhtml = escape_html(dp->d_name)) == NULL)
+ if ((escapedhtml = escape_html(dp->d_name)) == NULL) {
+ free(escapeduri);
goto fail;
+ }
if (dp->d_name[0] == '.' &&
!(dp->d_name[1] == '.' && dp->d_name[2] == '\0')) {
@@ -571,7 +571,7 @@ server_file_index(struct httpd *env, str
strchr(escapeduri, ':') != NULL ? "./" : "",
escapeduri, escapedhtml,
MAXIMUM(namewidth, 0), " ", tmstr, "-") == -1)
- skip = 1;
+ goto fail;
} else if (S_ISREG(subst.st_mode)) {
if (evbuffer_add_printf(evb,
"<a href=\"%s%s\">%s</a>%*s%s%20llu\n",
@@ -579,16 +579,16 @@ server_file_index(struct httpd *env, str
escapeduri, escapedhtml,
MAXIMUM(namewidth, 0), " ",
tmstr, subst.st_size) == -1)
- skip = 1;
+ goto fail;
}
free(escapeduri);
free(escapedhtml);
free(dp);
}
free(namelist);
+ namelist = NULL;
- if (skip ||
- evbuffer_add_printf(evb,
+ if (evbuffer_add_printf(evb,
"</pre>\n<hr>\n</body>\n</html>\n") == -1)
goto abort;
@@ -629,6 +629,9 @@ server_file_index(struct httpd *env, str
bufferevent_free(clt->clt_bev);
clt->clt_bev = NULL;
abort:
+ for (; i < namesize; i++)
+ free(namelist[i]);
+ free(namelist);
if (fd != -1)
close(fd);
if (evb != NULL)
Index: server_http.c
===================================================================
RCS file: /home/cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.150
diff -u -p -r1.150 server_http.c
--- server_http.c 2 Mar 2022 11:10:43 -0000 1.150
+++ server_http.c 14 Aug 2022 18:42:33 -0000
@@ -1777,14 +1777,13 @@ read_errdoc(const char *root, const char
free(path);
if (fstat(fd, &sb) < 0) {
log_warn("%s: stat", __func__);
+ close(fd);
return (NULL);
}
if ((ret = calloc(1, sb.st_size + 1)) == NULL)
fatal("calloc");
- if (sb.st_size == 0)
- return (ret);
- if (read(fd, ret, sb.st_size) != sb.st_size) {
+ if (sb.st_size != 0 && read(fd, ret, sb.st_size) != sb.st_size) {
log_warn("%s: read", __func__);
close(fd);
free(ret);
