Browse Source

unset F_CHUNKED on new Transfer-Encoding. Closes: #1016690 [CVE-2020-8287]

Christoph Biedl 1 year ago
parent
commit
6857b95dcf
2 changed files with 68 additions and 0 deletions
  1. 67 0
      debian/patches/CVE-2020-8287.patch
  2. 1 0
      debian/patches/series

+ 67 - 0
debian/patches/CVE-2020-8287.patch

@@ -0,0 +1,67 @@
+Subject: [PATCH] http: unset `F_CHUNKED` on new `Transfer-Encoding`
+Origin: Upstream PR (from nodejs) https://github.com/nodejs/http-parser/pull/530
+From: Fedor Indutny <fedor@indutny.com>
+Date: Wed, 18 Nov 2020 20:50:21 -0800
+Date: 2022-08-05
+
+Duplicate `Transfer-Encoding` header should be a treated as a single,
+but with original header values concatenated with a comma separator. In
+the light of this, even if the past `Transfer-Encoding` ended with
+`chunked`, we should be not let the `F_CHUNKED` to leak into the next
+header, because mere presence of another header indicates that `chunked`
+is not the last transfer-encoding token.
+
+CVE-ID: CVE-2020-8287
+PR-URL: https://github.com/nodejs-private/node-private/pull/235
+Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
+--- a/http_parser.c
++++ b/http_parser.c
+@@ -1344,6 +1344,13 @@
+               } else if (parser->index == sizeof(TRANSFER_ENCODING)-2) {
+                 parser->header_state = h_transfer_encoding;
+                 parser->uses_transfer_encoding = 1;
++
++                /* Multiple `Transfer-Encoding` headers should be treated as
++                 * one, but with values separate by a comma.
++                 *
++                 * See: https://tools.ietf.org/html/rfc7230#section-3.2.2
++                 */
++                parser->flags &= ~F_CHUNKED;
+               }
+               break;
+ 
+--- a/test.c
++++ b/test.c
+@@ -2154,6 +2154,32 @@
+   ,.body= "2\r\nOK\r\n0\r\n\r\n"
+   ,.num_chunks_complete= 0
+   }
++#define HTTP_200_DUPLICATE_TE_NOT_LAST_CHUNKED 30
++, {.name= "HTTP 200 response with `chunked` and duplicate Transfer-Encoding"
++  ,.type= HTTP_RESPONSE
++  ,.raw= "HTTP/1.1 200 OK\r\n"
++         "Transfer-Encoding: chunked\r\n"
++         "Transfer-Encoding: identity\r\n"
++         "\r\n"
++         "2\r\n"
++         "OK\r\n"
++         "0\r\n"
++         "\r\n"
++  ,.should_keep_alive= FALSE
++  ,.message_complete_on_eof= TRUE
++  ,.http_major= 1
++  ,.http_minor= 1
++  ,.status_code= 200
++  ,.response_status= "OK"
++  ,.content_length= -1
++  ,.num_headers= 2
++  ,.headers=
++    { { "Transfer-Encoding", "chunked" }
++    , { "Transfer-Encoding", "identity" }
++    }
++  ,.body= "2\r\nOK\r\n0\r\n\r\n"
++  ,.num_chunks_complete= 0
++  }
+ };
+ 
+ /* strnlen() is a POSIX.2008 addition. Can't rely on it being available so

+ 1 - 0
debian/patches/series

@@ -4,6 +4,7 @@ cherry-pick.v2.9.4-6-gd9275da.fix-wsign-compare-warning.patch
 cherry-pick.v2.9.4-7-g4b99e42.test-content-length-header-parsing.patch
 cherry-pick.v2.9.4-8-ge13b274.allow-content-length-and-transfer-encoding-chunked.patch
 cherry-pick.v2.9.4-9-g4f15b7d.fix-sizeof-http-parser-assert.patch
+CVE-2020-8287.patch
 
 # Debian-specific
 debian.improve-installation.patch