Download raw body.
httpd: imsg_get_fd and fix a use-after-free
This converts httpd to use imsg_get_fd().
While there fix a use-after-free on failure in config_getserver().
The problem is hit when get_data() fails for the variable-length values.
Shuffling the code a bit to insert the srv onto the lists later fixes the
problem.
Also do the same no-fd passing change in proc_forward_imsg() as done in
iked. Again nothing needs this so neuter this miss-feature.
--
:wq Claudio
Index: config.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/config.c,v
diff -u -p -r1.63 config.c
--- config.c 28 Dec 2022 21:30:16 -0000 1.63
+++ config.c 12 Dec 2023 16:46:54 -0000
@@ -626,6 +626,7 @@ config_getserver(struct httpd *env, stru
struct server_config srv_conf;
uint8_t *p = imsg->data;
size_t s;
+ int fd;
IMSG_SIZE_CHECK(imsg, &srv_conf);
memcpy(&srv_conf, p, sizeof(srv_conf));
@@ -634,6 +635,8 @@ config_getserver(struct httpd *env, stru
/* Reset these variables to avoid free'ing invalid pointers */
serverconfig_reset(&srv_conf);
+ fd = imsg_get_fd(imsg);
+
if ((IMSG_DATA_SIZE(imsg) - s) < (size_t)srv_conf.return_uri_len) {
log_debug("%s: invalid message length", __func__);
goto fail;
@@ -643,11 +646,11 @@ config_getserver(struct httpd *env, stru
if ((srv = server_byaddr((struct sockaddr *)
&srv_conf.ss, srv_conf.port)) != NULL) {
/* Add "host" to existing listening server */
- if (imsg->fd != -1) {
+ if (fd != -1) {
if (srv->srv_s == -1)
- srv->srv_s = imsg->fd;
+ srv->srv_s = fd;
else
- close(imsg->fd);
+ close(fd);
}
return (config_getserver_config(env, srv, imsg));
}
@@ -660,7 +663,7 @@ config_getserver(struct httpd *env, stru
goto fail;
memcpy(&srv->srv_conf, &srv_conf, sizeof(srv->srv_conf));
- srv->srv_s = imsg->fd;
+ srv->srv_s = fd;
if (config_getserver_auth(env, &srv->srv_conf) != 0)
goto fail;
@@ -668,14 +671,6 @@ config_getserver(struct httpd *env, stru
SPLAY_INIT(&srv->srv_clients);
TAILQ_INIT(&srv->srv_hosts);
- TAILQ_INSERT_TAIL(&srv->srv_hosts, &srv->srv_conf, entry);
- TAILQ_INSERT_TAIL(env->sc_servers, srv, srv_entry);
-
- DPRINTF("%s: %s %d configuration \"%s[%u]\", flags: %s", __func__,
- ps->ps_title[privsep_process], ps->ps_instance,
- srv->srv_conf.name, srv->srv_conf.id,
- printb_flags(srv->srv_conf.flags, SRVFLAG_BITS));
-
/*
* Get all variable-length values for the parent server.
*/
@@ -685,11 +680,19 @@ config_getserver(struct httpd *env, stru
goto fail;
}
+ TAILQ_INSERT_TAIL(&srv->srv_hosts, &srv->srv_conf, entry);
+ TAILQ_INSERT_TAIL(env->sc_servers, srv, srv_entry);
+
+ DPRINTF("%s: %s %d configuration \"%s[%u]\", flags: %s", __func__,
+ ps->ps_title[privsep_process], ps->ps_instance,
+ srv->srv_conf.name, srv->srv_conf.id,
+ printb_flags(srv->srv_conf.flags, SRVFLAG_BITS));
+
return (0);
fail:
- if (imsg->fd != -1)
- close(imsg->fd);
+ if (fd != -1)
+ close(fd);
if (srv != NULL)
serverconfig_free(&srv->srv_conf);
free(srv);
Index: logger.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/logger.c,v
diff -u -p -r1.24 logger.c
--- logger.c 27 Jan 2021 07:21:53 -0000 1.24
+++ logger.c 12 Dec 2023 16:41:02 -0000
@@ -144,9 +144,9 @@ logger_open_fd(struct imsg *imsg)
TAILQ_FOREACH(log, &log_files, log_entry) {
if (log->log_id == id) {
+ log->log_fd = imsg_get_fd(imsg);
DPRINTF("%s: received log fd %d, file %s",
- __func__, imsg->fd, log->log_name);
- log->log_fd = imsg->fd;
+ __func__, log->log_fd, log->log_name);
return (0);
}
}
Index: proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/proc.c,v
diff -u -p -r1.42 proc.c
--- proc.c 15 Feb 2023 20:44:01 -0000 1.42
+++ proc.c 12 Dec 2023 16:40:05 -0000
@@ -668,7 +668,7 @@ proc_dispatch(int fd, short event, void
case IMSG_CTL_PROCFD:
IMSG_SIZE_CHECK(&imsg, &pf);
memcpy(&pf, imsg.data, sizeof(pf));
- proc_accept(ps, imsg.fd, pf.pf_procid,
+ proc_accept(ps, imsg_get_fd(&imsg), pf.pf_procid,
pf.pf_instance);
break;
default:
@@ -799,7 +799,7 @@ proc_forward_imsg(struct privsep *ps, st
enum privsep_procid id, int n)
{
return (proc_compose_imsg(ps, id, n, imsg->hdr.type,
- imsg->hdr.peerid, imsg->fd, imsg->data, IMSG_DATA_SIZE(imsg)));
+ imsg->hdr.peerid, -1, imsg->data, IMSG_DATA_SIZE(imsg)));
}
struct imsgbuf *
httpd: imsg_get_fd and fix a use-after-free