From 3ca2365904062f883751a4099e2eb5e47ecf11b7 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 18 Sep 2020 07:43:38 +0200 Subject: [PATCH] BUG/MEDIUM: h2: report frame bits only for handled types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- include/haproxy/h2.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/haproxy/h2.h b/include/haproxy/h2.h index aa0edec93..e4eaccafe 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; }