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.
This commit is contained in:
Willy Tarreau 2026-05-04 13:51:36 +02:00
parent b6995d25d1
commit 7e09e2a916

View File

@ -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;