From 4b37de078bfa850ea3d08d02e23b912fd5f8c168 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Sun, 24 Nov 2019 10:34:39 +0100 Subject: BUG/MAJOR: h2: make header field name filtering stronger MIME-Version: 1.0 Content-Type: text/plain; charset=latin1 Content-Transfer-Encoding: 8bit Tim Düsterhus found that the amount of sanitization we perform on HTTP header field names received in H2 is insufficient. Currently we reject upper case letters as mandated by RFC7540#8.1.2, but section 10.3 also requires that intermediaries translating streams to HTTP/1 further refine the filtering to also reject invalid names (which means any name that doesn't match a token). There is a small trick here which is that the colon character used to start pseudo-header names doesn't match a token, so pseudo-header names fall into that category, thus we have to swap the pseudo-header name lookup with this check so that we only check from the second character (past the ':') in case of pseudo-header names. Another possibility could have been to perform this check only in the HTX-to-H1 trancoder but doing would still expose the configured rules and logs to such header names. This fix must be backported as far as 1.8 since this bug could be exploited and serve as the base for an attack. In 2.0 and earlier, functions h2_make_h1_request() and h2_make_h1_trailers() must also be adapted to sanitize requests coming in legacy mode. (cherry picked from commit 146f53ae7e97dbfe496d0445c2802dd0a30b0878) [wt: check added to h2_make_h1_request() and h2_make_h1_trailers()] Signed-off-by: Willy Tarreau (cherry picked from commit 5eaeec588d1de4f262f4ecffe0d39a36212965c3) Signed-off-by: Willy Tarreau (cherry picked from commit 8cf848ae60040620bd9293d912c3e3d68505379e) [wt: dropped HTX and trailers parts since absent in 1.8; include h1.h] Signed-off-by: Willy Tarreau --- src/h2.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/h2.c b/src/h2.c index 62b4241..e5351d7 100644 --- a/src/h2.c +++ b/src/h2.c @@ -30,7 +30,7 @@ #include #include #include - +#include /* Looks into for forbidden characters for header values (0x00, 0x0A, * 0x0D), starting at pointer which must be within . Returns @@ -168,12 +168,17 @@ int h2_make_h1_request(struct http_hdr *list, char *out, int osize, unsigned int } else { /* this can be any type of header */ - /* RFC7540#8.1.2: upper case not allowed in header field names */ - for (i = 0; i < list[idx].n.len; i++) - if ((uint8_t)(list[idx].n.ptr[i] - 'A') < 'Z' - 'A') - goto fail; - + /* RFC7540#8.1.2: upper case not allowed in header field names. + * #10.3: header names must be valid (i.e. match a token). + * For pseudo-headers we check from 2nd char and for other ones + * from the first char, because HTTP_IS_TOKEN() also excludes + * the colon. + */ phdr = h2_str_to_phdr(list[idx].n); + + for (i = !!phdr; i < list[idx].n.len; i++) + if ((uint8_t)(list[idx].n.ptr[i] - 'A') < 'Z' - 'A' || !HTTP_IS_TOKEN(list[idx].n.ptr[i])) + goto fail; } /* RFC7540#10.3: intermediaries forwarding to HTTP/1 must take care of -- 2.9.0