BUG/MAJOR: mux-h2: detect incomplete transfers on HEADERS frames as well

Checks are already made on H2 to detect inconsistencies between
advertised content-length and transferred data (excess of data or
premature END_STREAM flag on DATA frame). However, as found by
Martino Spagnuolo (r3verii), a subtle case remains: if the END_STREAM
appears on the HEADERS frame (i.e. a regular request for example),
then the check is not made. In this case it is possible to advertise
more contents than will really be transferred. If the other side uses
HTTP/1.1, and the server responds before the end of the transfer,
this means that the number of advertised bytes that will never be
transferred and that the server will drain will be taken from the
next request, effectively hiding a part of the header.

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.

The risk remains moderate given the low prevalence of "http-reuse never"
in production environments, and of idle servers.

The fix consists in detecting if advertised content-length remains when
processing an END_STREAM flag on a HEADERS frame. It also does it for
trailers, which turn out to be another way to abuse the bug. However it
takes great care not to break bodyless responses (204, 304 and responses
to HEAD requests) that may present a content-length that doesn't reflect
the presence of a body in the response.

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.

This must be backported to all stable versions.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 2ff7b2c2feaa54e4ca8263d5c86610947016bcb3..7183803ab3128ec7d13aaafc099bb3de961cd38f 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -6341,6 +6341,15 @@ next_frame:
                        TRACE_STATE("invalid interim response with ES flag", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_H2C_ERR|H2_EV_PROTO_ERR, h2c->conn);
                        goto fail;
                }
+               /* Note that bodyless only applies to responses, even when
+                * reported on the request (e.g. HEAD).
+                */
+               if ((msgf & H2_MSGF_BODY_CL) && *body_len > 0 &&
+                   (!(h2c->flags & H2_CF_IS_BACK) || !(*flags & H2_SF_BODYLESS_RESP))) {
+                       h2c_report_glitch(h2c, 1, "ES on HEADERS before end of content-length");
+                       TRACE_STATE("ES on HEADERS before end of content-length", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_H2C_ERR|H2_EV_PROTO_ERR, h2c->conn);
+                       goto fail;
+               }
                /* no more data are expected for this message */
                htx->flags |= HTX_FL_EOM;
                *flags |= H2_SF_ES_RCVD;