Re: Bug: RFC 7230 Section 3.3.3 violation in httpd header parsing enables TE.CL Request Smuggling

On Mon, Apr 13, 2026 at 02:13:18PM +0000, Ben Kallus wrote:
> > security impact of this is minimal.
> 
> I'm inclined to agree. Anyone processing POSTs with httpd is probably
> doing so with fastcgi anyway, which is currently (and has always been)
> broken.

You never responded to my patches I sent for your report. So this was not
pushed further.

Also your statement is incorrect. Processing POST requests works. Only
POST request using chunked encoding result in an error. The diff below
fixes that.

-- 
:wq Claudio

Index: httpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
diff -u -p -r1.129 httpd.conf.5
--- httpd.conf.5	18 Jan 2026 16:38:02 -0000	1.129
+++ httpd.conf.5	23 Jan 2026 12:27:41 -0000
@@ -392,6 +392,15 @@ This allows FastCGI server chroot to be 
 .It Ic param Ar variable value
 Sets a variable that will be sent to the FastCGI server.
 Each statement defines one variable.
+.It Ic pass Ic chunked
+Allow chunked uploads to be passed to the FastCGI server.
+By default data uploads with
+.Ar Transfer-Encoding: chunked
+are forbidden.
+By setting
+.Ic pass Ic chunked
+uploads with chunked encoding are accepted but because the CONTENT_LENGTH
+variable can not be set it violates the CGI speficfication.
 .El
 .Pp
 The FastCGI handler will be given the following variables by default:
Index: httpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
diff -u -p -r1.169 httpd.h
--- httpd.h	2 Mar 2026 19:24:58 -0000	1.169
+++ httpd.h	16 Mar 2026 14:59:56 -0000
@@ -108,7 +108,8 @@ enum httpchunk {
 	TOREAD_HTTP_CHUNK_LENGTH	= -3,
 	TOREAD_HTTP_CHUNK_TRAILER	= -4,
 	TOREAD_HTTP_NONE		= -5,
-	TOREAD_HTTP_RANGE		= TOREAD_HTTP_CHUNK_LENGTH
+	TOREAD_HTTP_RANGE		= TOREAD_HTTP_CHUNK_LENGTH,
+	TOREAD_HTTP_FINAL_CHUNK_TRAILER	= -6,
 };
 
 #if DEBUG
@@ -540,6 +541,7 @@ struct server_config {
 
 	struct server_fcgiparams fcgiparams;
 	int			 fcgistrip;
+	int			 fcgiallowchunked;
 	char			 errdocroot[HTTPD_ERRDOCROOT_MAX];
 
 	TAILQ_ENTRY(server_config) entry;
@@ -707,7 +709,7 @@ void	 server_file_error(struct buffereve
 
 /* server_fcgi.c */
 int	 server_fcgi(struct httpd *, struct client *);
-int	 fcgi_add_stdin(struct client *, struct evbuffer *);
+int	 fcgi_add_stdin(struct client *, const char *, size_t);
 
 /* httpd.c */
 void		 event_again(struct event *, int, short,
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
diff -u -p -r1.131 parse.y
--- parse.y	2 Mar 2026 19:24:58 -0000	1.131
+++ parse.y	16 Mar 2026 14:59:56 -0000
@@ -831,6 +831,16 @@ fcgiflags	: SOCKET STRING {
 			}
 			srv_conf->fcgistrip = $2;
 		}
+		| PASS STRING {
+			if (strcmp($2, "chunked") != 0) {
+				yyerror("Invalid token '%s' expected 'chunked'",
+				    $2);
+				free($2);
+				YYERROR;
+			}
+			srv_conf->fcgiallowchunked = 1;
+			free($2);
+		}
 		;
 
 connection	: CONNECTION '{' optnl conflags_l '}'
Index: server_fcgi.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
diff -u -p -r1.100 server_fcgi.c
--- server_fcgi.c	2 Mar 2026 19:24:58 -0000	1.100
+++ server_fcgi.c	16 Mar 2026 14:59:56 -0000
@@ -388,7 +388,7 @@ server_fcgi(struct httpd *env, struct cl
 		bufferevent_enable(clt->clt_bev, EV_READ);
 	} else {
 		bufferevent_disable(clt->clt_bev, EV_READ);
-		fcgi_add_stdin(clt, NULL);
+		fcgi_add_stdin(clt, NULL, 0);
 	}
 
 	if (strcmp(desc->http_version, "HTTP/1.1") == 0) {
@@ -414,7 +414,7 @@ server_fcgi(struct httpd *env, struct cl
 }
 
 int
-fcgi_add_stdin(struct client *clt, struct evbuffer *evbuf)
+fcgi_add_stdin(struct client *clt, const char *buf, size_t len)
 {
 	struct fcgi_record_header	h;
 
@@ -424,16 +424,16 @@ fcgi_add_stdin(struct client *clt, struc
 	h.id = htons(1);
 	h.padding_len = 0;
 
-	if (evbuf == NULL) {
+	if (len == 0) {
 		h.content_len = 0;
 		return bufferevent_write(clt->clt_srvbev, &h,
 		    sizeof(struct fcgi_record_header));
 	} else {
-		h.content_len = htons(EVBUFFER_LENGTH(evbuf));
+		h.content_len = htons(len);
 		if (bufferevent_write(clt->clt_srvbev, &h,
 		    sizeof(struct fcgi_record_header)) == -1)
 			return -1;
-		return bufferevent_write_buffer(clt->clt_srvbev, evbuf);
+		return bufferevent_write(clt->clt_srvbev, buf, len);
 	}
 	return (0);
 }
Index: server_http.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
diff -u -p -r1.161 server_http.c
--- server_http.c	2 Mar 2026 19:24:58 -0000	1.161
+++ server_http.c	16 Mar 2026 14:59:56 -0000
@@ -512,11 +512,11 @@ server_read_httpcontent(struct buffereve
 {
 	struct client		*clt = arg;
 	struct evbuffer		*src = EVBUFFER_INPUT(bev);
-	size_t			 size;
+	const char		*buf = EVBUFFER_DATA(src);
+	size_t			 size = EVBUFFER_LENGTH(src);
 
 	getmonotime(&clt->clt_tv_last);
 
-	size = EVBUFFER_LENGTH(src);
 	DPRINTF("%s: session %d: size %lu, to read %lld", __func__,
 	    clt->clt_id, size, clt->clt_toread);
 	if (!size)
@@ -524,21 +524,18 @@ server_read_httpcontent(struct buffereve
 
 	if (clt->clt_toread > 0) {
 		/* Read content data */
-		if ((off_t)size > clt->clt_toread) {
+		if ((off_t)size > clt->clt_toread)
 			size = clt->clt_toread;
-			if (fcgi_add_stdin(clt, src) == -1)
-				goto fail;
-			clt->clt_toread = 0;
-		} else {
-			if (fcgi_add_stdin(clt, src) == -1)
-				goto fail;
-			clt->clt_toread -= size;
-		}
+
+		if (fcgi_add_stdin(clt, buf, size) == -1)
+			goto fail;
+		evbuffer_drain(src, size);
+		clt->clt_toread -= size;
 		DPRINTF("%s: done, size %lu, to read %lld", __func__,
 		    size, clt->clt_toread);
 	}
 	if (clt->clt_toread == 0) {
-		fcgi_add_stdin(clt, NULL);
+		fcgi_add_stdin(clt, NULL, 0);
 		clt->clt_toread = TOREAD_HTTP_HEADER;
 		bufferevent_disable(bev, EV_READ);
 		bev->readcb = server_read_http;
@@ -561,13 +558,13 @@ server_read_httpchunks(struct buffereven
 {
 	struct client		*clt = arg;
 	struct evbuffer		*src = EVBUFFER_INPUT(bev);
+	const char		*buf = EVBUFFER_DATA(src);
+	size_t			 size = EVBUFFER_LENGTH(src);
 	char			*line;
 	long long		 llval;
-	size_t			 size;
 
 	getmonotime(&clt->clt_tv_last);
 
-	size = EVBUFFER_LENGTH(src);
 	DPRINTF("%s: session %d: size %lu, to read %lld", __func__,
 	    clt->clt_id, size, clt->clt_toread);
 	if (!size)
@@ -575,17 +572,15 @@ server_read_httpchunks(struct buffereven
 
 	if (clt->clt_toread > 0) {
 		/* Read chunk data */
-		if ((off_t)size > clt->clt_toread) {
+		if ((off_t)size > clt->clt_toread)
 			size = clt->clt_toread;
-			if (server_bufferevent_write_chunk(clt, src, size)
-			    == -1)
-				goto fail;
-			clt->clt_toread = 0;
-		} else {
-			if (server_bufferevent_write_buffer(clt, src) == -1)
-				goto fail;
-			clt->clt_toread -= size;
-		}
+		if (fcgi_add_stdin(clt, buf, size) == -1)
+			goto fail;
+		clt->clt_toread -= size;
+		evbuffer_drain(src, size);
+
+		if (clt->clt_toread == 0)
+			clt->clt_toread = TOREAD_HTTP_CHUNK_TRAILER;
 		DPRINTF("%s: done, size %lu, to read %lld", __func__,
 		    size, clt->clt_toread);
 	}
@@ -612,18 +607,15 @@ server_read_httpchunks(struct buffereven
 			return;
 		}
 
-		if (server_bufferevent_print(clt, line) == -1 ||
-		    server_bufferevent_print(clt, "\r\n") == -1) {
-			free(line);
-			goto fail;
-		}
 		free(line);
 
 		if ((clt->clt_toread = llval) == 0) {
 			DPRINTF("%s: last chunk", __func__);
-			clt->clt_toread = TOREAD_HTTP_CHUNK_TRAILER;
+			fcgi_add_stdin(clt, NULL, 0);
+			clt->clt_toread = TOREAD_HTTP_FINAL_CHUNK_TRAILER;
 		}
 		break;
+	case TOREAD_HTTP_FINAL_CHUNK_TRAILER:
 	case TOREAD_HTTP_CHUNK_TRAILER:
 		/* Last chunk is 0 bytes followed by trailer and empty line */
 		line = evbuffer_readln(src, NULL, EVBUFFER_EOL_CRLF_STRICT);
@@ -632,15 +624,15 @@ server_read_httpchunks(struct buffereven
 			bufferevent_enable(bev, EV_READ);
 			return;
 		}
-		if (server_bufferevent_print(clt, line) == -1 ||
-		    server_bufferevent_print(clt, "\r\n") == -1) {
-			free(line);
-			goto fail;
-		}
 		if (strlen(line) == 0) {
 			/* Switch to HTTP header mode */
-			clt->clt_toread = TOREAD_HTTP_HEADER;
-			bev->readcb = server_read_http;
+			if (clt->clt_toread == TOREAD_HTTP_CHUNK_TRAILER) {
+				clt->clt_toread = TOREAD_HTTP_CHUNK_LENGTH;
+			} else {
+				clt->clt_toread = TOREAD_HTTP_HEADER;
+				bufferevent_disable(bev, EV_READ);
+				bev->readcb = server_read_http;
+			}
 		}
 		free(line);
 		break;
@@ -648,8 +640,6 @@ server_read_httpchunks(struct buffereven
 		/* Chunk is terminated by an empty newline */
 		line = evbuffer_readln(src, NULL, EVBUFFER_EOL_CRLF_STRICT);
 		free(line);
-		if (server_bufferevent_print(clt, "\r\n") == -1)
-			goto fail;
 		clt->clt_toread = TOREAD_HTTP_CHUNK_LENGTH;
 		break;
 	}
@@ -1463,6 +1453,11 @@ server_response(struct httpd *httpd, str
 	if (clt->clt_toread > 0 && (size_t)clt->clt_toread >
 	    srv_conf->maxrequestbody) {
 		server_abort_http(clt, 413, "request body too large");
+		return (-1);
+	}
+	if (desc->http_chunked && !srv_conf->fcgiallowchunked) {
+		server_abort_http(clt, 400,
+		    "Transfer-Encoding: chunked not allowed");
 		return (-1);
 	}