From: Christopher Faulet <cfaulet@haproxy.com>
Date: Tue, 28 Sep 2021 07:50:07 +0000 (+0200)
Subject: BUG/MEDIUM: mux-h1/mux-fcgi: Reject messages with unknown transfer encoding
X-Git-Tag: v2.5-dev9~146
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=da3adebd06a85cacbdca577f03e433243c351fe9;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: mux-h1/mux-fcgi: Reject messages with unknown transfer encoding

HAproxy only handles "chunked" encoding internally. Because it is a gateway,
we stated it was not a problem if unknown encodings were applied on a
message because it is the recipient responsibility to accept the message or
not. And indeed, it is not a problem if both the client and the server
connections are using H1. However, Transfer-Encoding headers are dropped
from H2 messages. It is not a problem for chunk-encoded payload because
dechunking is performed during H1 parsing. But, for any other encodings, the
xferred H2 message is invalid.

It is also a problem for internal payload manipulations (lua,
filters...). Because the TE request headers are now sanitiezd, unsupported
encoding should not be used by servers. Thus it is only a problem for the
request messages. For this reason, such messages are now rejected. And if a
server decides to use an unknown encoding, the response will also be
rejected.

Note that it is pretty uncommon to use other encoding than "chunked" on the
request payload. So it is not necessary to backport it.

This patch should fix the issue #1301. No backport is needed.
---

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index 78705bdeff..3f127c78e5 100644
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -3347,6 +3347,20 @@ static size_t fcgi_strm_parse_headers(struct fcgi_strm *fstrm, struct h1m *h1m,
 		goto end;
 	}
 
+	/* Reject any message with an unknown transfer-encoding. In fact if any
+	 * encoding other than "chunked". A 422-Unprocessable-Content is
+	 * returned for an invalid request, a 502-Bad-Gateway for an invalid
+	 * response.
+	 */
+	if (h1m->flags & H1_MF_TE_OTHER) {
+		htx->flags |= HTX_FL_PARSING_ERROR;
+		TRACE_ERROR("Unknown transfer-encoding", FCGI_EV_RSP_DATA|FCGI_EV_RSP_HDRS|FCGI_EV_FSTRM_ERR, fstrm->fconn->conn, fstrm);
+		fcgi_strm_error(fstrm);
+		fcgi_strm_capture_bad_message(fstrm->fconn, fstrm, h1m, buf);
+		ret = 0;
+		goto end;
+	}
+
 	*ofs += ret;
   end:
 	TRACE_LEAVE(FCGI_EV_RSP_DATA|FCGI_EV_RSP_HDRS, fstrm->fconn->conn, fstrm, 0, (size_t[]){ret});
diff --git a/src/mux_h1.c b/src/mux_h1.c
index 47322cb3ed..dcfa3eef02 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -1420,6 +1420,22 @@ static size_t h1_handle_headers(struct h1s *h1s, struct h1m *h1m, struct htx *ht
 		goto end;
 	}
 
+	/* Reject any message with an unknown transfer-encoding. In fact if any
+	 * encoding other than "chunked". A 422-Unprocessable-Content is
+	 * returned for an invalid request, a 502-Bad-Gateway for an invalid
+	 * response.
+	 */
+	if (h1m->flags & H1_MF_TE_OTHER) {
+		h1s->flags |= H1S_F_PARSING_ERROR;
+		htx->flags |= HTX_FL_PARSING_ERROR;
+		if (!(h1m->flags & H1_MF_RESP))
+			h1s->h1c->errcode = 422;
+		TRACE_ERROR("Unknown transfer-encoding", H1_EV_RX_DATA|H1_EV_RX_HDRS|H1_EV_H1S_ERR, h1s->h1c->conn, h1s);
+		h1_capture_bad_message(h1s->h1c, h1s, h1m, buf);
+		ret = 0;
+		goto end;
+	}
+
 	/* If websocket handshake, search for the websocket key */
 	if ((h1m->flags & (H1_MF_CONN_UPG|H1_MF_UPG_WEBSOCKET)) ==
 	    (H1_MF_CONN_UPG|H1_MF_UPG_WEBSOCKET)) {