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 90834c562f029d96afad7ec1d4bccb1f39d1604c..3dd907e1637c9fee02879f28aa19573bdf6310f6 100644 (file)
--- 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 92ec96bfe19e30e565dad1173bba9441919dd24e..38b73cd955708279de0cf3ab766976da0a5ba4d2 100644 (file)
--- 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 85cd2f2e1dc39f00ad64e7a1c45b738814dd5570..2436292b209f8a3d5a1f35355d1439d7c793d437 100644 (file)
--- 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;