From: millert <millert@openbsd.org>
Date: Wed, 8 Nov 2023 19:19:10 +0000 (+0000)
Subject: Avoid a NULL dereference when handling a malformed fastcgi request.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=76ed904538b7966e735c4736a6e2cf7222ad67cf;p=openbsd

Avoid a NULL dereference when handling a malformed fastcgi request.

Rework the hack to avoid a use-after-free in the fastcgi code.
Since server_fcgi() can be called by server_read_httpcontent() we
can't set clt_fcgi_error to NULL.  Instead, we implement a simple
reference count to track when a fastcgi session is in progress to
avoid closing the http session prematurely on fastcgi error.
Based on a diff from and OK by tb@.  Reported by Ben Kallus.
---

diff --git a/usr.sbin/httpd/httpd.h b/usr.sbin/httpd/httpd.h
index 1e2ccb9d296..d2bf85f3c42 100644
--- a/usr.sbin/httpd/httpd.h
+++ b/usr.sbin/httpd/httpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: httpd.h,v 1.163 2023/07/12 12:37:27 tb Exp $	*/
+/*	$OpenBSD: httpd.h,v 1.164 2023/11/08 19:19:10 millert Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
@@ -350,6 +350,7 @@ struct client {
 	int			 clt_done;
 	int			 clt_chunk;
 	int			 clt_inflight;
+	int			 clt_fcgi_count;
 	struct range_data	 clt_ranges;
 	struct fcgi_data	 clt_fcgi;
 	const char		*clt_fcgi_error;
diff --git a/usr.sbin/httpd/server.c b/usr.sbin/httpd/server.c
index 5eeabb6ffd6..5d5063b6480 100644
--- a/usr.sbin/httpd/server.c
+++ b/usr.sbin/httpd/server.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server.c,v 1.128 2023/09/03 10:18:18 nicm Exp $	*/
+/*	$OpenBSD: server.c,v 1.129 2023/11/08 19:19:10 millert Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
@@ -1300,7 +1300,7 @@ server_close(struct client *clt, const char *msg)
 {
 	struct server		*srv = clt->clt_srv;
 
-	if (clt->clt_fcgi_error != NULL) {
+	if (clt->clt_fcgi_count-- > 0) {
 		clt->clt_fcgi_error = msg;
 		return;
 	}
diff --git a/usr.sbin/httpd/server_fcgi.c b/usr.sbin/httpd/server_fcgi.c
index d2c7d8a2076..70bd78c0820 100644
--- a/usr.sbin/httpd/server_fcgi.c
+++ b/usr.sbin/httpd/server_fcgi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server_fcgi.c,v 1.96 2023/07/12 12:37:28 tb Exp $	*/
+/*	$OpenBSD: server_fcgi.c,v 1.97 2023/11/08 19:19:10 millert Exp $	*/
 
 /*
  * Copyright (c) 2014 Florian Obser <florian@openbsd.org>
@@ -374,16 +374,15 @@ server_fcgi(struct httpd *env, struct client *clt)
 	if (clt->clt_toread != 0) {
 		/*
 		 * XXX - Work around UAF: server_read_httpcontent() can call
-		 * server_close(), normally freeing clt. If clt->clt_fcgi_error
-		 * changed, call server_close() via server_abort_http().
+		 * server_close(), normally freeing clt. If clt->clt_fcgi_count
+		 * reaches 0, call server_close() via server_abort_http().
 		 */
-		clt->clt_fcgi_error = "";
+		clt->clt_fcgi_count++;
 		server_read_httpcontent(clt->clt_bev, clt);
-		errstr = clt->clt_fcgi_error;
-		clt->clt_fcgi_error = NULL;
-		if (errstr[0] != '\0')
+		if (clt->clt_fcgi_count-- <= 0) {
+			errstr = clt->clt_fcgi_error;
 			goto fail;
-		errstr = NULL;
+		}
 		bufferevent_enable(clt->clt_bev, EV_READ);
 	} else {
 		bufferevent_disable(clt->clt_bev, EV_READ);