diff options
author | Ben Noordhuis <info@bnoordhuis.nl> | 2018-03-27 16:45:33 +0200 |
---|---|---|
committer | Myles Borins <mylesborins@google.com> | 2018-03-27 19:54:20 -0400 |
commit | ed64cc2be7bf967c04cb164db17d4ff6f462b0be (patch) | |
tree | eabfe8c9f2a5e8641faf5408922373a4fd54ae0d /deps | |
parent | d786d21f92f365beae216a16a58772a1f5cbf980 (diff) | |
download | node-new-ed64cc2be7bf967c04cb164db17d4ff6f462b0be.tar.gz |
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>
Diffstat (limited to 'deps')
-rw-r--r-- | deps/http_parser/http_parser.c | 19 | ||||
-rw-r--r-- | deps/http_parser/test.c | 21 |
2 files changed, 39 insertions, 1 deletions
diff --git a/deps/http_parser/http_parser.c b/deps/http_parser/http_parser.c index 7a9c688b1c..6522618671 100644 --- a/deps/http_parser/http_parser.c +++ b/deps/http_parser/http_parser.c @@ -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 @@ -1406,6 +1408,7 @@ reexecute: parser->flags |= F_CONTENTLENGTH; parser->content_length = ch - '0'; + parser->header_state = h_content_length_num; break; case h_connection: @@ -1493,10 +1496,18 @@ reexecute: 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); @@ -1519,6 +1530,12 @@ reexecute: 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++; diff --git a/deps/http_parser/test.c b/deps/http_parser/test.c index bc4e664f52..cb445cea86 100644 --- a/deps/http_parser/test.c +++ b/deps/http_parser/test.c @@ -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); |