From 3bd4bbdb9f54c18856aeb66b4b9f4a698973d3d3 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 12 Sep 2019 14:01:40 +0200 Subject: [PATCH] BUG/MEDIUM: http: also reject messages where "chunked" is missing from transfer-enoding Nathan Davison (@ndavison) reported that in legacy mode we don't correctly reject requests or responses featuring a transfer-encoding header missing the "chunked" value. As mandated in the protocol spec, the test verifies that "chunked" is the last one, but only does so when it is present. As such, "transfer-encoding: foobar" is not rejected, only "transfer-encoding: chunked, foobar" will be. The impact is limited, but if combined with "http-reuse always", it could be used as a help to construct a content smuggling attack against a vulnerable component employing a lenient parser which would ignore the content-length header as soon as it sees a transfer-encoding one, without even parsing it. In this case haproxy would fail to protect it. The fix consists in completing the existing checks to verify that "chunked" was present if any "transfer-encoding" header was met, otherwise either reject the request message or make the response end on a close. This fix is only for 2.0 and older versions as legacy mode was removed from 2.1. It should be backported to all maintained versions. (cherry picked from commit 196a7df44d8129d1adc795da020b722614d6a581) Signed-off-by: Christopher Faulet (cherry picked from commit 5513fcaa601dd344be548430fc1760dbedebf4f2) Signed-off-by: Willy Tarreau --- src/proto_http.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/proto_http.c b/src/proto_http.c index 411eb69899df..3c65606325e2 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -2110,6 +2110,10 @@ int http_wait_for_request(struct stream *s, struct channel *req, int an_bit) } } + /* "chunked" mandatory if transfer-encoding is used */ + if (ctx.idx && !(msg->flags & HTTP_MSGF_TE_CHNK)) + goto return_bad_req; + /* Chunked requests must have their content-length removed */ ctx.idx = 0; if (msg->flags & HTTP_MSGF_TE_CHNK) { @@ -5568,6 +5572,12 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) } } + /* "chunked" mandatory if transfer-encoding is used */ + if (ctx.idx && !(msg->flags & HTTP_MSGF_TE_CHNK)) { + use_close_only = 1; + msg->flags &= ~(HTTP_MSGF_TE_CHNK | HTTP_MSGF_XFER_LEN); + } + /* Chunked responses must have their content-length removed */ ctx.idx = 0; if (use_close_only || (msg->flags & HTTP_MSGF_TE_CHNK)) { -- 2.25.0