| 1 | From fc70ce08f5818a286fb5899a1bc3aff5965a745e Mon Sep 17 00:00:00 2001
|
| 2 | From: Fedor Indutny <[email protected]>
|
| 3 | Date: Wed, 18 Nov 2020 20:50:21 -0800
|
| 4 | Subject: [PATCH] http: unset `F_CHUNKED` on new `Transfer-Encoding`
|
| 5 |
|
| 6 | Duplicate `Transfer-Encoding` header should be a treated as a single,
|
| 7 | but with original header values concatenated with a comma separator. In
|
| 8 | the light of this, even if the past `Transfer-Encoding` ended with
|
| 9 | `chunked`, we should be not let the `F_CHUNKED` to leak into the next
|
| 10 | header, because mere presence of another header indicates that `chunked`
|
| 11 | is not the last transfer-encoding token.
|
| 12 |
|
| 13 | CVE-ID: CVE-2020-8287
|
| 14 | PR-URL: https://github.com/nodejs-private/node-private/pull/235
|
| 15 | Reviewed-By: Fedor Indutny <[email protected]>
|
| 16 | ---
|
| 17 | http_parser.c | 7 +++++++
|
| 18 | test.c | 26 ++++++++++++++++++++++++++
|
| 19 | 2 files changed, 33 insertions(+)
|
| 20 |
|
| 21 | diff --git a/http_parser.c b/http_parser.c
|
| 22 | index 9be003e7322..e9b2b9e83b9 100644
|
| 23 | |
| 24 | |
| 25 | @@ -1344,6 +1344,13 @@ size_t http_parser_execute (http_parser *parser,
|
| 26 | } else if (parser->index == sizeof(TRANSFER_ENCODING)-2) {
|
| 27 | parser->header_state = h_transfer_encoding;
|
| 28 | parser->uses_transfer_encoding = 1;
|
| 29 | +
|
| 30 | + /* Multiple `Transfer-Encoding` headers should be treated as
|
| 31 | + * one, but with values separate by a comma.
|
| 32 | + *
|
| 33 | + * See: https://tools.ietf.org/html/rfc7230#section-3.2.2
|
| 34 | + */
|
| 35 | + parser->flags &= ~F_CHUNKED;
|
| 36 | }
|
| 37 | break;
|
| 38 |
|
| 39 | diff --git a/test.c b/test.c
|
| 40 | index 3f7c77b3494..2e5a9ebd678 100644
|
| 41 | |
| 42 | |
| 43 | @@ -2154,6 +2154,32 @@ const struct message responses[] =
|
| 44 | ,.body= "2\r\nOK\r\n0\r\n\r\n"
|
| 45 | ,.num_chunks_complete= 0
|
| 46 | }
|
| 47 | +#define HTTP_200_DUPLICATE_TE_NOT_LAST_CHUNKED 30
|
| 48 | +, {.name= "HTTP 200 response with `chunked` and duplicate Transfer-Encoding"
|
| 49 | + ,.type= HTTP_RESPONSE
|
| 50 | + ,.raw= "HTTP/1.1 200 OK\r\n"
|
| 51 | + "Transfer-Encoding: chunked\r\n"
|
| 52 | + "Transfer-Encoding: identity\r\n"
|
| 53 | + "\r\n"
|
| 54 | + "2\r\n"
|
| 55 | + "OK\r\n"
|
| 56 | + "0\r\n"
|
| 57 | + "\r\n"
|
| 58 | + ,.should_keep_alive= FALSE
|
| 59 | + ,.message_complete_on_eof= TRUE
|
| 60 | + ,.http_major= 1
|
| 61 | + ,.http_minor= 1
|
| 62 | + ,.status_code= 200
|
| 63 | + ,.response_status= "OK"
|
| 64 | + ,.content_length= -1
|
| 65 | + ,.num_headers= 2
|
| 66 | + ,.headers=
|
| 67 | + { { "Transfer-Encoding", "chunked" }
|
| 68 | + , { "Transfer-Encoding", "identity" }
|
| 69 | + }
|
| 70 | + ,.body= "2\r\nOK\r\n0\r\n\r\n"
|
| 71 | + ,.num_chunks_complete= 0
|
| 72 | + }
|
| 73 | };
|
| 74 |
|
| 75 | /* strnlen() is a POSIX.2008 addition. Can't rely on it being available so
|