Skip to content

Commit 01da95f

Browse files
committed
Reject interior blanks in Content-Length value.
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: nodejs-private/security#178 PR-URL: nodejs-private/http-parser-private#1 Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
1 parent 214fa6f commit 01da95f

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

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++;

test.c

+21
Original file line numberDiff line numberDiff line change
@@ -4147,6 +4147,27 @@ main (void)
41474147
test_invalid_header_field_token_error(HTTP_RESPONSE);
41484148
test_invalid_header_field_content_error(HTTP_RESPONSE);
41494149

4150+
test_simple_type(
4151+
"POST / HTTP/1.1\r\n"
4152+
"Content-Length: 42 \r\n" // Note the surrounding whitespace.
4153+
"\r\n",
4154+
HPE_OK,
4155+
HTTP_REQUEST);
4156+
4157+
test_simple_type(
4158+
"POST / HTTP/1.1\r\n"
4159+
"Content-Length: 4 2\r\n"
4160+
"\r\n",
4161+
HPE_INVALID_CONTENT_LENGTH,
4162+
HTTP_REQUEST);
4163+
4164+
test_simple_type(
4165+
"POST / HTTP/1.1\r\n"
4166+
"Content-Length: 13 37\r\n"
4167+
"\r\n",
4168+
HPE_INVALID_CONTENT_LENGTH,
4169+
HTTP_REQUEST);
4170+
41504171
//// RESPONSES
41514172

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

0 commit comments

Comments
 (0)