|
 |
55719b |
From c39167dc260953184a1ccd45292947808b94507d Mon Sep 17 00:00:00 2001
|
|
|
55719b |
From: Ben Noordhuis <info@bnoordhuis.nl>
|
|
|
55719b |
Date: Tue, 27 Mar 2018 16:45:33 +0200
|
|
|
55719b |
Subject: [PATCH] deps: reject interior blanks in Content-Length
|
|
|
55719b |
MIME-Version: 1.0
|
|
|
55719b |
Content-Type: text/plain; charset=UTF-8
|
|
|
55719b |
Content-Transfer-Encoding: 8bit
|
|
|
55719b |
|
|
|
55719b |
Original commit message follows:
|
|
|
55719b |
|
|
|
55719b |
Before this commit `Content-Length: 4 2` was accepted as a valid
|
|
|
55719b |
header and recorded as `parser->content_length = 42`. Now it is
|
|
|
55719b |
a parse error that fails with error `HPE_INVALID_CONTENT_LENGTH`.
|
|
|
55719b |
|
|
|
55719b |
Downstream users that inspect `parser->content_length` and naively
|
|
|
55719b |
parse the string value using `strtoul()` might get confused by the
|
|
|
55719b |
discrepancy between the two values. Resolve that by simply not
|
|
|
55719b |
letting it happen.
|
|
|
55719b |
|
|
|
55719b |
Fixes: https://github.com/nodejs-private/security/issues/178
|
|
|
55719b |
PR-URL: https://github.com/nodejs-private/http-parser-private/pull/1
|
|
|
55719b |
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
|
|
|
55719b |
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
|
55719b |
Reviewed-By: Evan Lucas <evanlucas@me.com>
|
|
|
55719b |
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
|
|
|
55719b |
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
|
55719b |
Reviewed-By: Rod Vagg <rod@vagg.org>
|
|
|
55719b |
---
|
|
|
55719b |
deps/http_parser/http_parser.c | 19 ++++++++++++++++++-
|
|
|
55719b |
deps/http_parser/test.c | 21 +++++++++++++++++++++
|
|
|
55719b |
2 files changed, 39 insertions(+), 1 deletion(-)
|
|
|
55719b |
|
|
|
55719b |
diff --git a/deps/http_parser/http_parser.c b/deps/http_parser/http_parser.c
|
|
|
55719b |
index 7a9c688b1ca7..6522618671d0 100644
|
|
|
55719b |
--- a/deps/http_parser/http_parser.c
|
|
|
55719b |
+++ b/deps/http_parser/http_parser.c
|
|
|
55719b |
@@ -370,6 +370,8 @@ enum header_states
|
|
|
55719b |
|
|
|
55719b |
, h_connection
|
|
|
55719b |
, h_content_length
|
|
|
55719b |
+ , h_content_length_num
|
|
|
55719b |
+ , h_content_length_ws
|
|
|
55719b |
, h_transfer_encoding
|
|
|
55719b |
, h_upgrade
|
|
|
55719b |
|
|
|
55719b |
@@ -1406,6 +1408,7 @@ size_t http_parser_execute (http_parser *parser,
|
|
|
55719b |
|
|
|
55719b |
parser->flags |= F_CONTENTLENGTH;
|
|
|
55719b |
parser->content_length = ch - '0';
|
|
|
55719b |
+ parser->header_state = h_content_length_num;
|
|
|
55719b |
break;
|
|
|
55719b |
|
|
|
55719b |
case h_connection:
|
|
|
55719b |
@@ -1493,10 +1496,18 @@ size_t http_parser_execute (http_parser *parser,
|
|
|
55719b |
break;
|
|
|
55719b |
|
|
|
55719b |
case h_content_length:
|
|
|
55719b |
+ if (ch == ' ') break;
|
|
|
55719b |
+ h_state = h_content_length_num;
|
|
|
55719b |
+ /* FALLTHROUGH */
|
|
|
55719b |
+
|
|
|
55719b |
+ case h_content_length_num:
|
|
|
55719b |
{
|
|
|
55719b |
uint64_t t;
|
|
|
55719b |
|
|
|
55719b |
- if (ch == ' ') break;
|
|
|
55719b |
+ if (ch == ' ') {
|
|
|
55719b |
+ h_state = h_content_length_ws;
|
|
|
55719b |
+ break;
|
|
|
55719b |
+ }
|
|
|
55719b |
|
|
|
55719b |
if (UNLIKELY(!IS_NUM(ch))) {
|
|
|
55719b |
SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);
|
|
|
55719b |
@@ -1519,6 +1530,12 @@ size_t http_parser_execute (http_parser *parser,
|
|
|
55719b |
break;
|
|
|
55719b |
}
|
|
|
55719b |
|
|
|
55719b |
+ case h_content_length_ws:
|
|
|
55719b |
+ if (ch == ' ') break;
|
|
|
55719b |
+ SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);
|
|
|
55719b |
+ parser->header_state = h_state;
|
|
|
55719b |
+ goto error;
|
|
|
55719b |
+
|
|
|
55719b |
/* Transfer-Encoding: chunked */
|
|
|
55719b |
case h_matching_transfer_encoding_chunked:
|
|
|
55719b |
parser->index++;
|
|
|
55719b |
diff --git a/deps/http_parser/test.c b/deps/http_parser/test.c
|
|
|
55719b |
index bc4e664f5253..cb445cea8607 100644
|
|
|
55719b |
--- a/deps/http_parser/test.c
|
|
|
55719b |
+++ b/deps/http_parser/test.c
|
|
|
55719b |
@@ -4168,6 +4168,27 @@ main (void)
|
|
|
55719b |
test_invalid_header_field_token_error(HTTP_RESPONSE);
|
|
|
55719b |
test_invalid_header_field_content_error(HTTP_RESPONSE);
|
|
|
55719b |
|
|
|
55719b |
+ test_simple_type(
|
|
|
55719b |
+ "POST / HTTP/1.1\r\n"
|
|
|
55719b |
+ "Content-Length: 42 \r\n" // Note the surrounding whitespace.
|
|
|
55719b |
+ "\r\n",
|
|
|
55719b |
+ HPE_OK,
|
|
|
55719b |
+ HTTP_REQUEST);
|
|
|
55719b |
+
|
|
|
55719b |
+ test_simple_type(
|
|
|
55719b |
+ "POST / HTTP/1.1\r\n"
|
|
|
55719b |
+ "Content-Length: 4 2\r\n"
|
|
|
55719b |
+ "\r\n",
|
|
|
55719b |
+ HPE_INVALID_CONTENT_LENGTH,
|
|
|
55719b |
+ HTTP_REQUEST);
|
|
|
55719b |
+
|
|
|
55719b |
+ test_simple_type(
|
|
|
55719b |
+ "POST / HTTP/1.1\r\n"
|
|
|
55719b |
+ "Content-Length: 13 37\r\n"
|
|
|
55719b |
+ "\r\n",
|
|
|
55719b |
+ HPE_INVALID_CONTENT_LENGTH,
|
|
|
55719b |
+ HTTP_REQUEST);
|
|
|
55719b |
+
|
|
|
55719b |
//// RESPONSES
|
|
|
55719b |
|
|
|
55719b |
test_simple_type("HTP/1.1 200 OK\r\n\r\n", HPE_INVALID_VERSION, HTTP_RESPONSE);
|