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