Skip to content

Commit c39167d

Browse files
bnoordhuisMylesBorins
authored andcommitted
deps: reject interior blanks in Content-Length
Original commit message follows: Before this commit `Content-Length: 4 2` was accepted as a valid header and recorded as `parser->content_length = 42`. Now it is a parse error that fails with error `HPE_INVALID_CONTENT_LENGTH`. Downstream users that inspect `parser->content_length` and naively parse the string value using `strtoul()` might get confused by the discrepancy between the two values. Resolve that by simply not letting it happen. Fixes: https://github.com/nodejs-private/security/issues/178 PR-URL: https://github.com/nodejs-private/http-parser-private/pull/1 Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rod Vagg <rod@vagg.org>
1 parent 3bc15a6 commit c39167d

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

deps/http_parser/http_parser.c

+18-1
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,8 @@ enum header_states
370370

371371
, h_connection
372372
, h_content_length
373+
, h_content_length_num
374+
, h_content_length_ws
373375
, h_transfer_encoding
374376
, h_upgrade
375377

@@ -1406,6 +1408,7 @@ size_t http_parser_execute (http_parser *parser,
14061408

14071409
parser->flags |= F_CONTENTLENGTH;
14081410
parser->content_length = ch - '0';
1411+
parser->header_state = h_content_length_num;
14091412
break;
14101413

14111414
case h_connection:
@@ -1493,10 +1496,18 @@ size_t http_parser_execute (http_parser *parser,
14931496
break;
14941497

14951498
case h_content_length:
1499+
if (ch == ' ') break;
1500+
h_state = h_content_length_num;
1501+
/* FALLTHROUGH */
1502+
1503+
case h_content_length_num:
14961504
{
14971505
uint64_t t;
14981506

1499-
if (ch == ' ') break;
1507+
if (ch == ' ') {
1508+
h_state = h_content_length_ws;
1509+
break;
1510+
}
15001511

15011512
if (UNLIKELY(!IS_NUM(ch))) {
15021513
SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);
@@ -1519,6 +1530,12 @@ size_t http_parser_execute (http_parser *parser,
15191530
break;
15201531
}
15211532

1533+
case h_content_length_ws:
1534+
if (ch == ' ') break;
1535+
SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);
1536+
parser->header_state = h_state;
1537+
goto error;
1538+
15221539
/* Transfer-Encoding: chunked */
15231540
case h_matching_transfer_encoding_chunked:
15241541
parser->index++;

deps/http_parser/test.c

+21
Original file line numberDiff line numberDiff line change
@@ -4168,6 +4168,27 @@ main (void)
41684168
test_invalid_header_field_token_error(HTTP_RESPONSE);
41694169
test_invalid_header_field_content_error(HTTP_RESPONSE);
41704170

4171+
test_simple_type(
4172+
"POST / HTTP/1.1\r\n"
4173+
"Content-Length: 42 \r\n" // Note the surrounding whitespace.
4174+
"\r\n",
4175+
HPE_OK,
4176+
HTTP_REQUEST);
4177+
4178+
test_simple_type(
4179+
"POST / HTTP/1.1\r\n"
4180+
"Content-Length: 4 2\r\n"
4181+
"\r\n",
4182+
HPE_INVALID_CONTENT_LENGTH,
4183+
HTTP_REQUEST);
4184+
4185+
test_simple_type(
4186+
"POST / HTTP/1.1\r\n"
4187+
"Content-Length: 13 37\r\n"
4188+
"\r\n",
4189+
HPE_INVALID_CONTENT_LENGTH,
4190+
HTTP_REQUEST);
4191+
41714192
//// RESPONSES
41724193

41734194
test_simple_type("HTP/1.1 200 OK\r\n\r\n", HPE_INVALID_VERSION, HTTP_RESPONSE);

0 commit comments

Comments
 (0)