From 631c7e866522f8a11c9218ffed7a95d0d6b96b5e Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Mon, 27 Sep 2021 09:47:03 +0200 Subject: [PATCH] MEDIUM: h1: Force close mode for invalid uses of T-E header Transfer-Encoding header is not supported in HTTP/1.0. However, softwares dealing with HTTP/1.0 and HTTP/1.1 messages may accept it and transfer it. When a Content-Length header is also provided, it must be ignored. Unfortunately, this may lead to vulnerabilities (request smuggling or response splitting) if an intermediary is only implementing HTTP/1.0. Because it may ignore Transfer-Encoding header and only handle Content-Length one. To avoid any security issues, when Transfer-Encoding and Content-Length headers are found in a message, the close mode is forced. The same is performed for HTTP/1.0 message with a Transfer-Encoding header only. This change is conform to what it is described in the last HTTP/1.1 draft. See also httpwg/http-core#879. Note that Content-Length header is also removed from any incoming messages if a Transfer-Encoding header is found. However it is not true (not yet) for responses generated by HAProxy. --- src/h1.c | 6 ++++++ src/mux_h1.c | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/h1.c b/src/h1.c index 4b13cab86d..e7334fac30 100644 --- a/src/h1.c +++ b/src/h1.c @@ -938,9 +938,15 @@ int h1_headers_to_hdr_list(char *start, const char *stop, state = H1_MSG_DATA; if (h1m->flags & H1_MF_XFER_ENC) { if (h1m->flags & H1_MF_CLEN) { + /* T-E + C-L: force close and remove C-L */ + h1m->flags |= H1_MF_CONN_CLO; h1m->flags &= ~H1_MF_CLEN; hdr_count = http_del_hdr(hdr, ist("content-length")); } + else if (!(h1m->flags & H1_MF_VER_11)) { + /* T-E + HTTP/1.0: force close */ + h1m->flags |= H1_MF_CONN_CLO; + } if (h1m->flags & H1_MF_CHNK) state = H1_MSG_CHUNK_SIZE; diff --git a/src/mux_h1.c b/src/mux_h1.c index a391ab1284..a627f19859 100644 --- a/src/mux_h1.c +++ b/src/mux_h1.c @@ -1995,12 +1995,22 @@ static size_t h1_process_mux(struct h1c *h1c, struct buffer *buf, size_t count) last_lf: h1m->state = H1_MSG_LAST_LF; if (!(h1s->flags & H1S_F_HAVE_O_CONN)) { - /* If the reply comes from haproxy while the request is - * not finished, we force the connection close. */ if ((chn_htx->flags & HTX_FL_PROXY_RESP) && h1s->req.state != H1_MSG_DONE) { + /* If the reply comes from haproxy while the request is + * not finished, we force the connection close. */ h1s->flags = (h1s->flags & ~H1S_F_WANT_MSK) | H1S_F_WANT_CLO; TRACE_STATE("force close mode (resp)", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s); } + else if ((h1m->flags & (H1_MF_XFER_ENC|H1_MF_CLEN)) == (H1_MF_XFER_ENC|H1_MF_CLEN)) { + /* T-E + C-L: force close */ + h1s->flags = (h1s->flags & ~H1S_F_WANT_MSK) | H1S_F_WANT_CLO; + TRACE_STATE("force close mode (T-E + C-L)", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s); + } + else if ((h1m->flags & (H1_MF_VER_11|H1_MF_XFER_ENC)) == H1_MF_XFER_ENC) { + /* T-E + HTTP/1.0: force close */ + h1s->flags = (h1s->flags & ~H1S_F_WANT_MSK) | H1S_F_WANT_CLO; + TRACE_STATE("force close mode (T-E + HTTP/1.0)", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s); + } /* the conn_mode must be processed. So do it */ n = ist("connection"); -- 2.39.5