From: Willy Tarreau Date: Wed, 9 Aug 2023 09:02:34 +0000 (+0200) Subject: BUG/MINOR: http: skip leading zeroes in content-length values X-Git-Tag: v2.9-dev3~43 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=22731762d9fe2c98d9e6c3942b1568266b23c69f;p=thirdparty%2Fhaproxy.git BUG/MINOR: http: skip leading zeroes in content-length values Ben Kallus also noticed that we preserve leading zeroes on content-length values. While this is totally valid, it would be safer to at least trim them before passing the value, because a bogus server written to parse using "strtol(value, NULL, 0)" could inadvertently take a leading zero as a prefix for an octal value. While there is not much that can be done to protect such servers in general (e.g. lack of check for overflows etc), at least it's quite cheap to make sure the transmitted value is normalized and not taken for an octal one. This is not really a bug, rather a missed opportunity to sanitize the input, but is marked as a bug so that we don't forget to backport it to stable branches. A combined regtest was added to h1or2_to_h1c which already validates end-to-end syntax consistency on aggregate headers. --- diff --git a/reg-tests/http-rules/h1or2_to_h1c.vtc b/reg-tests/http-rules/h1or2_to_h1c.vtc index 90834c562f..3dd907e163 100644 --- a/reg-tests/http-rules/h1or2_to_h1c.vtc +++ b/reg-tests/http-rules/h1or2_to_h1c.vtc @@ -26,11 +26,11 @@ server s1 { -body "This is a body" expect req.method == "GET" - expect req.http.fe-sl1-crc == 992395575 - expect req.http.fe-sl2-crc == 1270056220 + expect req.http.fe-sl1-crc == 1874847043 + expect req.http.fe-sl2-crc == 1142278307 expect req.http.fe-hdr-crc == 1719311923 - expect req.http.be-sl1-crc == 2604236007 - expect req.http.be-sl2-crc == 4181358964 + expect req.http.be-sl1-crc == 3455320059 + expect req.http.be-sl2-crc == 2509326257 expect req.http.be-hdr-crc == 3634102538 } -repeat 2 -start @@ -51,6 +51,7 @@ haproxy h1 -conf { http-request set-var(req.path) path http-request set-var(req.query) query http-request set-var(req.param) url_param(qs_arg) + http-request set-var(req.cl) req.fhdr(content-length) http-request set-header sl1 "sl1: " @@ -63,8 +64,10 @@ haproxy h1 -conf { http-request set-header sl1 "%[req.fhdr(sl1)] method=<%[var(req.method)]>; uri=<%[var(req.uri)]>; path=<%[var(req.path)]>;" http-request set-header sl1 "%[req.fhdr(sl1)] query=<%[var(req.query)]>; param=<%[var(req.param)]>" + http-request set-header sl1 "%[req.fhdr(sl1)] cl=<%[var(req.cl)]>" http-request set-header sl2 "%[req.fhdr(sl2)] method=<%[method]>; uri=<%[url]>; path=<%[path]>; " http-request set-header sl2 "%[req.fhdr(sl2)] query=<%[query]>; param=<%[url_param(qs_arg)]>" + http-request set-header sl2 "%[req.fhdr(sl2)] cl=<%[req.fhdr(content-length)]>" http-request set-header hdr "%[req.fhdr(hdr)] hdr1=<%[req.hdr(hdr1)]>; fhdr1=<%[req.fhdr(hdr1)]>;" http-request set-header hdr "%[req.fhdr(hdr)] hdr2=<%[req.hdr(hdr2)]>; fhdr2=<%[req.fhdr(hdr2)]>;" http-request set-header hdr "%[req.fhdr(hdr)] hdr3=<%[req.hdr(hdr3)]>; fhdr3=<%[req.fhdr(hdr3)]>;" @@ -118,6 +121,7 @@ haproxy h1 -conf { http-request set-var(req.path) path http-request set-var(req.query) query http-request set-var(req.param) url_param(qs_arg) + http-request set-var(req.cl) req.fhdr(content-length) http-request set-header sl1 "sl1: " @@ -130,8 +134,10 @@ haproxy h1 -conf { http-request set-header sl1 "%[req.fhdr(sl1)] method=<%[var(req.method)]>; uri=<%[var(req.uri)]>; path=<%[var(req.path)]>;" http-request set-header sl1 "%[req.fhdr(sl1)] query=<%[var(req.query)]>; param=<%[var(req.param)]>" + http-request set-header sl1 "%[req.fhdr(sl1)] cl=<%[var(req.cl)]>" http-request set-header sl2 "%[req.fhdr(sl2)] method=<%[method]>; uri=<%[url]>; path=<%[path]>; " http-request set-header sl2 "%[req.fhdr(sl2)] query=<%[query]>; param=<%[url_param(QS_arg,,i)]>" + http-request set-header sl2 "%[req.fhdr(sl2)] cl=<%[req.fhdr(content-length)]>" http-request set-header hdr "%[req.fhdr(hdr)] hdr1=<%[req.hdr(hdr1)]>; fhdr1=<%[req.fhdr(hdr1)]>;" http-request set-header hdr "%[req.fhdr(hdr)] hdr2=<%[req.hdr(hdr2)]>; fhdr2=<%[req.fhdr(hdr2)]>;" http-request set-header hdr "%[req.fhdr(hdr)] hdr3=<%[req.hdr(hdr3)]>; fhdr3=<%[req.fhdr(hdr3)]>;" @@ -169,6 +175,7 @@ client c1h1 -connect ${h1_feh1_sock} { txreq \ -req GET \ -url /path/to/file.extension?qs_arg=qs_value \ + -hdr "content-length: 000, 00" \ -hdr "hdr1: val1" \ -hdr "hdr2: val2a" \ -hdr "hdr2: val2b" \ @@ -203,6 +210,7 @@ client c1h2 -connect ${h1_feh2_sock} { -req GET \ -scheme "https" \ -url /path/to/file.extension?qs_arg=qs_value \ + -hdr "content-length" "000, 00" \ -hdr "hdr1" "val1" \ -hdr "hdr2" " val2a" \ -hdr "hdr2" " val2b" \ diff --git a/src/h1.c b/src/h1.c index 92ec96bfe1..38b73cd955 100644 --- a/src/h1.c +++ b/src/h1.c @@ -58,6 +58,14 @@ int h1_parse_cont_len_header(struct h1m *h1m, struct ist *value) goto fail; break; } + + if (unlikely(!cl && n > word.ptr)) { + /* There was a leading zero before this digit, + * let's trim it. + */ + word.ptr = n; + } + if (unlikely(cl > ULLONG_MAX / 10ULL)) goto fail; /* multiply overflow */ cl = cl * 10ULL; diff --git a/src/http.c b/src/http.c index 85cd2f2e1d..2436292b20 100644 --- a/src/http.c +++ b/src/http.c @@ -731,6 +731,14 @@ int http_parse_cont_len_header(struct ist *value, unsigned long long *body_len, goto fail; break; } + + if (unlikely(!cl && n > word.ptr)) { + /* There was a leading zero before this digit, + * let's trim it. + */ + word.ptr = n; + } + if (unlikely(cl > ULLONG_MAX / 10ULL)) goto fail; /* multiply overflow */ cl = cl * 10ULL;