Fetch: limit decoded chunked response bodies

Previously, fetch allowed decoded chunked bodies to reach ten times
max_response_body_size.

The fix is to apply max_response_body_size to decoded chunks.

diff --git a/nginx/ngx_js_http.c b/nginx/ngx_js_http.c
index 4d88fe2293c909d06aa6d07234c68fba9355e70d..e7b0e8a76e74326b54b4ac5606767362983283d9 100644 (file)
--- a/nginx/ngx_js_http.c
+++ b/nginx/ngx_js_http.c
@@ -55,7 +55,7 @@ static ngx_int_t ngx_js_http_parse_status_line(ngx_js_http_parse_t *hp,
 static ngx_int_t ngx_js_http_parse_header_line(ngx_js_http_parse_t *hp,
     ngx_buf_t *b);
 static ngx_int_t ngx_js_http_parse_chunked(ngx_js_http_chunk_parse_t *hcp,
-    ngx_buf_t *b, njs_chb_t *chain);
+    ngx_buf_t *b, njs_chb_t *chain, size_t max_size);
 
 static void ngx_js_fetch_append_request_headers(njs_chb_t *chain,
     ngx_js_request_t *request, njs_bool_t is_proxy);
@@ -1038,7 +1038,13 @@ ngx_js_http_process_body(ngx_js_http_t *http)
 
     if (http->body == NGX_JS_HTTP_BODY_CHUNKED) {
         rc = ngx_js_http_parse_chunked(&http->http_chunk_parse, b,
-                                       &http->response.chain);
+                                       &http->response.chain,
+                                       http->max_response_body_size);
+        if (rc == NGX_DECLINED) {
+            ngx_js_http_error(http, "http response body is too large");
+            return NGX_ERROR;
+        }
+
         if (rc == NGX_ERROR) {
             ngx_js_http_error(http, "invalid http chunked response");
             return NGX_ERROR;
@@ -1061,11 +1067,6 @@ ngx_js_http_process_body(ngx_js_http_t *http)
             return NGX_DONE;
         }
 
-        if (size > http->max_response_body_size * 10) {
-            ngx_js_http_error(http, "very large http chunked response");
-            return NGX_ERROR;
-        }
-
         b->pos = http->http_chunk_parse.pos;
 
     } else {
@@ -1560,23 +1561,28 @@ ngx_size_is_sufficient(cs)                                                    \
 
 static ngx_int_t
 ngx_js_http_chunk_buffer(ngx_js_http_chunk_parse_t *hcp, ngx_buf_t *b,
-    njs_chb_t *chain)
+    njs_chb_t *chain, size_t max_size)
 {
-    size_t  size;
+    size_t  size, n;
 
     size = b->last - hcp->pos;
+    n = ngx_min(hcp->chunk_size, size);
+
+    if (n > max_size || chain->total_size > max_size - n) {
+        return NGX_DECLINED;
+    }
 
     if (hcp->chunk_size < size) {
-        njs_chb_append(chain, hcp->pos, hcp->chunk_size);
-        hcp->pos += hcp->chunk_size;
+        njs_chb_append(chain, hcp->pos, n);
+        hcp->pos += n;
 
         return NGX_JS_HTTP_CHUNK_END;
     }
 
-    njs_chb_append(chain, hcp->pos, size);
-    hcp->pos += size;
+    njs_chb_append(chain, hcp->pos, n);
+    hcp->pos += n;
 
-    hcp->chunk_size -= size;
+    hcp->chunk_size -= n;
 
     if (hcp->chunk_size == 0) {
         return NGX_JS_HTTP_CHUNK_ON_BORDER;
@@ -1588,7 +1594,7 @@ ngx_js_http_chunk_buffer(ngx_js_http_chunk_parse_t *hcp, ngx_buf_t *b,
 
 static ngx_int_t
 ngx_js_http_parse_chunked(ngx_js_http_chunk_parse_t *hcp,
-    ngx_buf_t *b, njs_chb_t *chain)
+    ngx_buf_t *b, njs_chb_t *chain, size_t max_size)
 {
     u_char     c, ch;
     ngx_int_t  rc;
@@ -1612,8 +1618,8 @@ ngx_js_http_parse_chunked(ngx_js_http_chunk_parse_t *hcp,
          * to preserve hcp->pos and to not touch memory.
          */
         if (state == sw_chunk) {
-            rc = ngx_js_http_chunk_buffer(hcp, b, chain);
-            if (rc == NGX_ERROR) {
+            rc = ngx_js_http_chunk_buffer(hcp, b, chain, max_size);
+            if (rc == NGX_ERROR || rc == NGX_DECLINED) {
                 return rc;
             }
 

diff --git a/nginx/t/js_fetch.t b/nginx/t/js_fetch.t
index 80a9859098bf6b50ee984181d8325119e8f51020..0f82e565880e3af199ae342f9711fcd2ee5a59ce 100644 (file)
--- a/nginx/t/js_fetch.t
+++ b/nginx/t/js_fetch.t
@@ -362,8 +362,11 @@ $t->write_file('test.js', <<EOF);
     }
 
     async function framing(r) {
+        var opts = {max_response_body_size: 128};
+
         try {
-            var reply = await ngx.fetch(`http://127.0.0.1:$p2/\${r.args.loc}`);
+            var reply = await ngx.fetch(`http://127.0.0.1:$p2/\${r.args.loc}`,
+                                        opts);
             var body = await reply.text();
             r.return(200, `ok:\${body.length}`);
 
@@ -508,7 +511,7 @@ EOF
 
 $t->try_run('no njs.fetch');
 
-$t->plan(54);
+$t->plan(58);
 
 $t->run_daemon(\&http_daemon, port(8082));
 $t->waitforsocket('127.0.0.1:' . port(8082));
@@ -604,6 +607,16 @@ like(http_get('/framing?loc=no_body/304_large'), qr/200 OK.*ok:0$/s,
 like(http_get('/framing?loc=length'),
        qr/501.*prematurely closed connection/s,
        'fetch incomplete fixed-length response');
+like(http_get('/framing?loc=chunked/exact'), qr/200 OK.*ok:128$/s,
+       'fetch chunked exact limit');
+like(http_get('/framing?loc=chunked/over'),
+       qr/501.*http response body is too large/s,
+       'fetch chunked response too large');
+like(http_get('/framing?loc=chunked/multi'), qr/200 OK.*ok:128$/s,
+       'fetch chunked multi-part limit');
+like(http_get('/framing?loc=chunked/multi_over'),
+       qr/501.*http response body is too large/s,
+       'fetch chunked multi-part response too large');
 like(http_get('/header_iter?loc=duplicate_header_large'),
        qr/\["A:a","B:a","C:a","D:a","E:a","F:a","G:a","H:a","Moo:a, ?b"]$/s,
        'fetch header duplicate large');
@@ -939,6 +952,26 @@ sub http_daemon {
 
                    print $client  "0" . CRLF . CRLF;
 
+               } elsif ($uri =~ m#^/chunked/(exact|over)$#) {
+                       my $size = ($1 eq 'exact') ? 128 : 129;
+                       print $client
+                               "HTTP/1.1 200 OK" . CRLF .
+                               "Transfer-Encoding: chunked" . CRLF .
+                               "Connection: close" . CRLF .
+                               CRLF . sprintf("%x", $size) . CRLF . ("X" x $size)
+                               . CRLF . "0" . CRLF . CRLF;
+
+               } elsif ($uri =~ m#^/chunked/(multi|multi_over)$#) {
+                       my $last = ($1 eq 'multi') ? 64 : 65;
+                       print $client
+                               "HTTP/1.1 200 OK" . CRLF .
+                               "Transfer-Encoding: chunked" . CRLF .
+                               "Connection: close" . CRLF .
+                               CRLF . "40" . CRLF . ("X" x 64) . CRLF;
+                       select undef, undef, undef, 0.01;
+                       print $client sprintf("%x", $last) . CRLF . ("X" x $last)
+                               . CRLF . "0" . CRLF . CRLF;
+
                }
        }
 }