Skip to content

Commit

Permalink
deps: reject interior blanks in Content-Length
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
bnoordhuis authored and MylesBorins committed Mar 27, 2018
1 parent 3bc15a6 commit c39167d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
19 changes: 18 additions & 1 deletion deps/http_parser/http_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,8 @@ enum header_states

, h_connection
, h_content_length
, h_content_length_num
, h_content_length_ws
, h_transfer_encoding
, h_upgrade

Expand Down Expand Up @@ -1406,6 +1408,7 @@ size_t http_parser_execute (http_parser *parser,

parser->flags |= F_CONTENTLENGTH;
parser->content_length = ch - '0';
parser->header_state = h_content_length_num;
break;

case h_connection:
Expand Down Expand Up @@ -1493,10 +1496,18 @@ size_t http_parser_execute (http_parser *parser,
break;

case h_content_length:
if (ch == ' ') break;
h_state = h_content_length_num;
/* FALLTHROUGH */

case h_content_length_num:
{
uint64_t t;

if (ch == ' ') break;
if (ch == ' ') {
h_state = h_content_length_ws;
break;
}

if (UNLIKELY(!IS_NUM(ch))) {
SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);
Expand All @@ -1519,6 +1530,12 @@ size_t http_parser_execute (http_parser *parser,
break;
}

case h_content_length_ws:
if (ch == ' ') break;
SET_ERRNO(HPE_INVALID_CONTENT_LENGTH);
parser->header_state = h_state;
goto error;

/* Transfer-Encoding: chunked */
case h_matching_transfer_encoding_chunked:
parser->index++;
Expand Down
21 changes: 21 additions & 0 deletions deps/http_parser/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -4168,6 +4168,27 @@ main (void)
test_invalid_header_field_token_error(HTTP_RESPONSE);
test_invalid_header_field_content_error(HTTP_RESPONSE);

test_simple_type(
"POST / HTTP/1.1\r\n"
"Content-Length: 42 \r\n" // Note the surrounding whitespace.
"\r\n",
HPE_OK,
HTTP_REQUEST);

test_simple_type(
"POST / HTTP/1.1\r\n"
"Content-Length: 4 2\r\n"
"\r\n",
HPE_INVALID_CONTENT_LENGTH,
HTTP_REQUEST);

test_simple_type(
"POST / HTTP/1.1\r\n"
"Content-Length: 13 37\r\n"
"\r\n",
HPE_INVALID_CONTENT_LENGTH,
HTTP_REQUEST);

//// RESPONSES

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

0 comments on commit c39167d

Please sign in to comment.