Merge pull request #81 from h2o/kazuho/chunk-overhead

Detect and reject excess overhead in chunked encoding

picohttpparser.c

@@ -545,6 +545,8 @@ ssize_t phr_decode_chunked(struct phr_chunked_decoder *decoder, char *buf, size_
     size_t dst = 0, src = 0, bufsz = *_bufsz;
     ssize_t ret = -2; /* incomplete */
 
+    decoder->_total_read += bufsz;
+
     while (1) {
         switch (decoder->_state) {
         case CHUNKED_IN_CHUNK_SIZE:
@@ -664,6 +666,12 @@ ssize_t phr_decode_chunked(struct phr_chunked_decoder *decoder, char *buf, size_
     if (dst != src)
         memmove(buf + dst, buf + src, bufsz - src);
     *_bufsz = dst;
+    /* if incomplete but the overhead of the chunked encoding is >=100KB and >80%, signal an error */
+    if (ret == -2) {
+        decoder->_total_overhead += bufsz - dst;
+        if (decoder->_total_overhead >= 100 * 1024 && decoder->_total_read - decoder->_total_overhead < decoder->_total_read / 4)
+            ret = -1;
+    }
     return ret;
 }
 

picohttpparser.h

@@ -27,6 +27,7 @@
 #ifndef picohttpparser_h
 #define picohttpparser_h
 
+#include <stdint.h>
 #include <sys/types.h>
 
 #ifdef _MSC_VER
@@ -64,6 +65,8 @@ struct phr_chunked_decoder {
     char consume_trailer;       /* if trailing headers should be consumed */
     char _hex_count;
     char _state;
+    uint64_t _total_read;
+    uint64_t _total_overhead;
 };
 
 /* the function rewrites the buffer given as (buf, bufsz) removing the chunked-

test.c

@@ -458,6 +458,53 @@ static void test_chunked_leftdata(void)
 #undef NEXT_REQ
 }
 
+static ssize_t do_test_chunked_overhead(size_t chunk_len, size_t chunk_count, const char *extra)
+{
+    struct phr_chunked_decoder dec = {0};
+    char buf[1024];
+    size_t bufsz;
+    ssize_t ret;
+
+    for (size_t i = 0; i < chunk_count; ++i) {
+        /* build and feed the chunk header */
+        bufsz = (size_t)sprintf(buf, "%zx%s\r\n", chunk_len, extra);
+        if ((ret = phr_decode_chunked(&dec, buf, &bufsz)) != -2)
+            goto Exit;
+        assert(bufsz == 0);
+        /* build and feed the chunk boby */
+        memset(buf, 'A', chunk_len);
+        bufsz = chunk_len;
+        if ((ret = phr_decode_chunked(&dec, buf, &bufsz)) != -2)
+            goto Exit;
+        assert(bufsz == chunk_len);
+        /* build and feed the chunk end (CRLF) */
+        strcpy(buf, "\r\n");
+        bufsz = 2;
+        if ((ret = phr_decode_chunked(&dec, buf, &bufsz)) != -2)
+            goto Exit;
+        assert(bufsz == 0);
+    }
+
+    /* build and feed the end chunk */
+    strcpy(buf, "0\r\n\r\n");
+    bufsz = 5;
+    ret = phr_decode_chunked(&dec, buf, &bufsz);
+    assert(bufsz == 0);
+
+Exit:
+    return ret;
+}
+
+static void test_chunked_overhead(void)
+{
+    ok(do_test_chunked_overhead(100, 10000, "") == 2 /* consume trailer is not set */);
+    ok(do_test_chunked_overhead(10, 100000, "") == 2 /* consume trailer is not set */);
+    ok(do_test_chunked_overhead(1, 1000000, "") == -1);
+
+    ok(do_test_chunked_overhead(10, 100000, "; tiny=1") == 2 /* consume trailer is not set */);
+    ok(do_test_chunked_overhead(10, 100000, "; large=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") == -1);
+}
+
 int main(void)
 {
     long pagesize = sysconf(_SC_PAGESIZE);
@@ -474,6 +521,7 @@ int main(void)
     subtest("chunked", test_chunked);
     subtest("chunked-consume-trailer", test_chunked_consume_trailer);
     subtest("chunked-leftdata", test_chunked_leftdata);
+    subtest("chunked-overhead", test_chunked_overhead);
 
     munmap(inputbuf - pagesize * 2, pagesize * 3);