From 7e09e2a916af400f8cde8dabf649adf7fc1f6600 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 4 May 2026 13:51:36 +0200 Subject: [PATCH] BUG/MAJOR: mux-h2: preset MSGF_BODY_CL on H2_SF_DATA_CLEN in h2c_dec_hdrs() Commit d12edebe4a ("BUG/MAJOR: mux-h2: detect incomplete transfers on HEADERS frames as well") tried to enforce strict matching between advertised content-length and transferred data when dealing with ES on a headers frame. It purposely arranged the code so that it would cover both headers and trailers. The problem is, in h2c_dec_hdrs() we preset message flags (msgf) based on the current state and knowledge related to the stream being processed, then we pass these flags to the headers parser and use their final state to perform some extra checks. MSGF_BODY_CL was set by the parsers themselves when processing a content-length header, but when parsing a trailers frame, it will not be set, and due to this the matching between the remaining expected content-length and the transferred data is not verified, so the fix above doesn't work for trailers. This patch sets MSGF_BODY_CL to the same value as H2_SF_DATA_CLEN so that during headers it remains zero, but it matches what was learned during headers when processing trailers. It is sufficient to re-enable the check that was attempted in the commit above. The impact remains the same as the one indicated in the commit above: in practice this can be used to force subsequent requests to fail, or when running with "http-reuse never" or when running with a totally idle server, to perform a request smuggling by constructing specially crafted request pairs where the first one is used to trigger an early response and hide parts of or all headers of the second one, to instead use a second embedded one that was not subject to analysis. As such, the risk remains moderate given the low prevalence of "http-reuse never" in production environments, and of idle servers. Again, a temporary alternative to the fix is to disable HTTP/2 by specifying "alpn http/1.1" on "bind" lines, and adding "option disable-h2-upgrade" in HTTP frontends. Many thanks to Pratham Gupta / alchemy1729 for spotting and analyzing this problem, and for providing a lightweight reproducer to illustrate the problem! This fix must be backported to all versions where the fix above was backported (i.e. all). Note that it depends on this previous commit otherwise trailers will always break: BUG/MEDIUM: mux-h2: fix the body_len to check when parsing request trailers As a side note, it's worth noting that these temporary message flags have reached a level of pain and fragility that really warrants a complete rework. Ideally we should have a pair of such flags in the h2s (one per direction) and the callers of the parsers should point to them so that they are always up to date. And by having generic HTTP flags instead of H2, we could better unify the h1/h2/h3/fcgi processors (and maybe avoid some HTX conversion). One flag could even indicate that trailers are being parsed (since they're last) so as to ease this detection down the chain. --- src/mux_h2.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/mux_h2.c b/src/mux_h2.c index 06326397a..1c720a44d 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -6287,6 +6287,9 @@ next_frame: * to convert 200 response to 101 htx response */ msgf |= (*flags & H2_SF_EXT_CONNECT_SENT) ? H2_MSGF_EXT_CONNECT: 0; + /* when dealing with trailers, we need to check the content-length */ + msgf |= (*flags & H2_SF_DATA_CLEN) ? H2_MSGF_BODY_CL : 0; + if (*flags & H2_SF_HEADERS_RCVD) goto trailers;