From: Willy Tarreau Date: Fri, 18 Sep 2020 05:43:38 +0000 (+0200) Subject: BUG/MEDIUM: h2: report frame bits only for handled types X-Git-Tag: v2.3-dev5~10 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3ca2365904062f883751a4099e2eb5e47ecf11b7;p=thirdparty%2Fhaproxy.git BUG/MEDIUM: h2: report frame bits only for handled types As part of his GREASE experiments on Chromium, Bence Béky reported in https://lists.w3.org/Archives/Public/ietf-http-wg/2020JulSep/0202.html and https://bugs.chromium.org/p/chromium/issues/detail?id=1127060 that a certain combination of frame type and frame flags was causing an error on app.slack.com. It turns out that it's haproxy that is causing this issue because the frame type is wrongly assumed to support padding, the frame flags indicate padding is present, and the frame is too short for this, resulting in an error. The reason why only some frame types are affected is due to the frame type being used in a bit shift to match against a mask, and where the 5 lower bits of the frame type only are used to compute the frame bit. If the resulting frame bit matches a DATA, HEADERS or PUSH_PROMISE frame bit, then padding support is assumed and the test is enforced, resulting in a PROTOCOL_ERROR or FRAME_SIZE_ERROR depending on the payload size. We must never match any such bit for unsupported frame types so let's add a check for this. This must be backported as far as 1.8. Thanks to Cooper Bethea for providing enough context to help narrow the issue down and to Bence Béky for creating a simple reproducer. --- diff --git a/include/haproxy/h2.h b/include/haproxy/h2.h index aa0edec936..e4eaccafef 100644 --- a/include/haproxy/h2.h +++ b/include/haproxy/h2.h @@ -212,6 +212,8 @@ int h2_make_htx_trailers(struct http_hdr *list, struct htx *htx); /* returns a bit corresponding to the frame type */ static inline unsigned int h2_ft_bit(enum h2_ft ft) { + if (ft >= H2_FT_ENTRIES) + return 0; return 1U << ft; }