diff --git a/deps/http_parser/http_parser.c b/deps/http_parser/http_parser.c index 55d7716228b770..0b2990970e87f3 100644 --- a/deps/http_parser/http_parser.c +++ b/deps/http_parser/http_parser.c @@ -387,6 +387,8 @@ enum http_host_state (IS_ALPHANUM(c) || (c) == '.' || (c) == '-' || (c) == '_') #endif +#define IS_HEADER_CHAR(ch) \ + (ch == CR || ch == LF || ch == 9 || (ch > 31 && ch != 127)) #define start_state (parser->type == HTTP_REQUEST ? s_start_req : s_start_res) @@ -590,6 +592,8 @@ size_t http_parser_execute (http_parser *parser, const char *url_mark = 0; const char *body_mark = 0; + const unsigned char lenient = parser->lenient_http_headers; + /* We're in an error state. Don't bother doing anything. */ if (HTTP_PARSER_ERRNO(parser) != HPE_OK) { return 0; @@ -1311,7 +1315,12 @@ size_t http_parser_execute (http_parser *parser, || c != CONTENT_LENGTH[parser->index]) { parser->header_state = h_general; } else if (parser->index == sizeof(CONTENT_LENGTH)-2) { - parser->header_state = h_content_length; + if (parser->flags & F_CONTENTLENGTH) { + SET_ERRNO(HPE_UNEXPECTED_CONTENT_LENGTH); + goto error; + } + parser->header_state = h_content_length; + parser->flags |= F_CONTENTLENGTH; } break; @@ -1457,6 +1466,11 @@ size_t http_parser_execute (http_parser *parser, goto reexecute_byte; } + if (!lenient && !IS_HEADER_CHAR(ch)) { + SET_ERRNO(HPE_INVALID_HEADER_TOKEN); + goto error; + } + c = LOWER(ch); switch (parser->header_state) { @@ -1541,7 +1555,10 @@ size_t http_parser_execute (http_parser *parser, case s_header_almost_done: { - STRICT_CHECK(ch != LF); + if (ch != LF) { + SET_ERRNO(HPE_LF_EXPECTED); + goto error; + } parser->state = s_header_value_lws; @@ -1585,6 +1602,14 @@ size_t http_parser_execute (http_parser *parser, break; } + /* Cannot use chunked encoding and a content-length header together + per the HTTP specification. */ + if ((parser->flags & F_CHUNKED) && + (parser->flags & F_CONTENTLENGTH)) { + SET_ERRNO(HPE_UNEXPECTED_CONTENT_LENGTH); + goto error; + } + parser->state = s_headers_done; /* Set this here so that on_headers_complete() callbacks can see it */ diff --git a/deps/http_parser/http_parser.h b/deps/http_parser/http_parser.h index 7908ad03bbbf75..7cb3b552573764 100644 --- a/deps/http_parser/http_parser.h +++ b/deps/http_parser/http_parser.h @@ -25,7 +25,7 @@ extern "C" { #endif #define HTTP_PARSER_VERSION_MAJOR 1 -#define HTTP_PARSER_VERSION_MINOR 0 +#define HTTP_PARSER_VERSION_MINOR 1 #include #if defined(_WIN32) && !defined(__MINGW32__) && (!defined(_MSC_VER) || _MSC_VER<1600) @@ -137,6 +137,7 @@ enum flags , F_TRAILING = 1 << 3 , F_UPGRADE = 1 << 4 , F_SKIPBODY = 1 << 5 + , F_CONTENTLENGTH = 1 << 6 }; @@ -176,6 +177,8 @@ enum flags XX(INVALID_HEADER_TOKEN, "invalid character in header") \ XX(INVALID_CONTENT_LENGTH, \ "invalid character in content-length header") \ + XX(UNEXPECTED_CONTENT_LENGTH, \ + "unexpected content-length header") \ XX(INVALID_CHUNK_SIZE, \ "invalid character in chunk size header") \ XX(INVALID_CONSTANT, "invalid constant string") \ @@ -207,10 +210,11 @@ enum http_errno { struct http_parser { /** PRIVATE **/ unsigned char type : 2; /* enum http_parser_type */ - unsigned char flags : 6; /* F_* values from 'flags' enum; semi-public */ + unsigned char flags : 7; /* F_* values from 'flags' enum; semi-public */ unsigned char state; /* enum state from http_parser.c */ - unsigned char header_state; /* enum header_state from http_parser.c */ - unsigned char index; /* index into current matcher */ + unsigned char header_state : 7; /* enum header_state from http_parser.c */ + unsigned char index : 7; /* index into current matcher */ + unsigned char lenient_http_headers : 1; uint32_t nread; /* # bytes read in various scenarios */ uint64_t content_length; /* # bytes in body (0 if no Content-Length header) */ diff --git a/deps/http_parser/test.c b/deps/http_parser/test.c index 46d817bb387d1d..76575fc414c2cb 100644 --- a/deps/http_parser/test.c +++ b/deps/http_parser/test.c @@ -2651,6 +2651,156 @@ test_simple (const char *buf, enum http_errno err_expected) } } +void +test_invalid_header_content (int req, const char* str) +{ + http_parser parser; + http_parser_init(&parser, req ? HTTP_REQUEST : HTTP_RESPONSE); + size_t parsed; + const char *buf; + buf = req ? + "GET / HTTP/1.1\r\n" : + "HTTP/1.1 200 OK\r\n"; + parsed = http_parser_execute(&parser, &settings_null, buf, strlen(buf)); + assert(parsed == strlen(buf)); + + buf = str; + size_t buflen = strlen(buf); + + parsed = http_parser_execute(&parser, &settings_null, buf, buflen); + if (parsed != buflen) { + assert(HTTP_PARSER_ERRNO(&parser) == HPE_INVALID_HEADER_TOKEN); + return; + } + + fprintf(stderr, + "\n*** Error expected but none in invalid header content test ***\n"); + abort(); +} + +void +test_invalid_header_field_content_error (int req) +{ + test_invalid_header_content(req, "Foo: F\01ailure"); + test_invalid_header_content(req, "Foo: B\02ar"); +} + +void +test_invalid_header_field (int req, const char* str) +{ + http_parser parser; + http_parser_init(&parser, req ? HTTP_REQUEST : HTTP_RESPONSE); + size_t parsed; + const char *buf; + buf = req ? + "GET / HTTP/1.1\r\n" : + "HTTP/1.1 200 OK\r\n"; + parsed = http_parser_execute(&parser, &settings_null, buf, strlen(buf)); + assert(parsed == strlen(buf)); + + buf = str; + size_t buflen = strlen(buf); + + parsed = http_parser_execute(&parser, &settings_null, buf, buflen); + if (parsed != buflen) { + assert(HTTP_PARSER_ERRNO(&parser) == HPE_INVALID_HEADER_TOKEN); + return; + } + + fprintf(stderr, + "\n*** Error expected but none in invalid header token test ***\n"); + abort(); +} + +void +test_invalid_header_field_token_error (int req) +{ + test_invalid_header_field(req, "Fo@: Failure"); + test_invalid_header_field(req, "Foo\01\test: Bar"); +} + +void +test_double_content_length_error (int req) +{ + http_parser parser; + http_parser_init(&parser, req ? HTTP_REQUEST : HTTP_RESPONSE); + size_t parsed; + const char *buf; + buf = req ? + "GET / HTTP/1.1\r\n" : + "HTTP/1.1 200 OK\r\n"; + parsed = http_parser_execute(&parser, &settings_null, buf, strlen(buf)); + assert(parsed == strlen(buf)); + + buf = "Content-Length: 0\r\nContent-Length: 1\r\n\r\n"; + size_t buflen = strlen(buf); + + parsed = http_parser_execute(&parser, &settings_null, buf, buflen); + if (parsed != buflen) { + assert(HTTP_PARSER_ERRNO(&parser) == HPE_UNEXPECTED_CONTENT_LENGTH); + return; + } + + fprintf(stderr, + "\n*** Error expected but none in double content-length test ***\n"); + abort(); +} + +void +test_chunked_content_length_error (int req) +{ + http_parser parser; + http_parser_init(&parser, req ? HTTP_REQUEST : HTTP_RESPONSE); + size_t parsed; + const char *buf; + buf = req ? + "GET / HTTP/1.1\r\n" : + "HTTP/1.1 200 OK\r\n"; + parsed = http_parser_execute(&parser, &settings_null, buf, strlen(buf)); + assert(parsed == strlen(buf)); + + buf = "Transfer-Encoding: chunked\r\nContent-Length: 1\r\n\r\n"; + size_t buflen = strlen(buf); + + parsed = http_parser_execute(&parser, &settings_null, buf, buflen); + if (parsed != buflen) { + assert(HTTP_PARSER_ERRNO(&parser) == HPE_UNEXPECTED_CONTENT_LENGTH); + return; + } + + fprintf(stderr, + "\n*** Error expected but none in chunked content-length test ***\n"); + abort(); +} + +void +test_header_cr_no_lf_error (int req) +{ + http_parser parser; + http_parser_init(&parser, req ? HTTP_REQUEST : HTTP_RESPONSE); + size_t parsed; + const char *buf; + buf = req ? + "GET / HTTP/1.1\r\n" : + "HTTP/1.1 200 OK\r\n"; + parsed = http_parser_execute(&parser, &settings_null, buf, strlen(buf)); + assert(parsed == strlen(buf)); + + buf = "Foo: 1\rBar: 1\r\n\r\n"; + size_t buflen = strlen(buf); + + parsed = http_parser_execute(&parser, &settings_null, buf, buflen); + if (parsed != buflen) { + assert(HTTP_PARSER_ERRNO(&parser) == HPE_LF_EXPECTED); + return; + } + + fprintf(stderr, + "\n*** Error expected but none in header whitespace test ***\n"); + abort(); +} + + void test_header_overflow_error (int req) { @@ -3048,6 +3198,19 @@ main (void) test_header_content_length_overflow_error(); test_chunk_content_length_overflow_error(); + //// HEADER FIELD CONDITIONS + test_double_content_length_error(HTTP_REQUEST); + test_chunked_content_length_error(HTTP_REQUEST); + test_header_cr_no_lf_error(HTTP_REQUEST); + test_invalid_header_field_token_error(HTTP_REQUEST); + test_invalid_header_field_content_error(HTTP_REQUEST); + test_double_content_length_error(HTTP_RESPONSE); + test_chunked_content_length_error(HTTP_RESPONSE); + test_header_cr_no_lf_error(HTTP_RESPONSE); + test_invalid_header_field_token_error(HTTP_RESPONSE); + test_invalid_header_field_content_error(HTTP_RESPONSE); + + //// RESPONSES for (i = 0; i < response_count; i++) { diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 0a261b003e629f..be72a145e954eb 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -24,6 +24,7 @@ #include "v8.h" #include "node.h" #include "node_buffer.h" +#include "node_revert.h" #include /* strdup() */ #if !defined(_MSC_VER) @@ -546,6 +547,8 @@ class Parser : public ObjectWrap { void Init(enum http_parser_type type) { http_parser_init(&parser_, type); + /* Allow the strict http header parsing to be reverted */ + parser_.lenient_http_headers = IsReverted(REVERT_CVE_2016_2216) ? 1 : 0; url_.Reset(); num_fields_ = 0; num_values_ = 0; diff --git a/src/node_revert.h b/src/node_revert.h index bbde63e6f6bb75..78699b75009c25 100644 --- a/src/node_revert.h +++ b/src/node_revert.h @@ -32,8 +32,8 @@ * For *master* this list should always be empty! * **/ -#define REVERSIONS(XX) -// XX(CVE_2016_PEND, "CVE-2016-PEND", "Vulnerability Title") +#define REVERSIONS(XX) \ + XX(CVE_2016_2216, "CVE-2016-2216", "Strict HTTP Header Parsing") namespace node { diff --git a/test/simple/test-http-client-reject-chunked-with-content-length.js b/test/simple/test-http-client-reject-chunked-with-content-length.js new file mode 100644 index 00000000000000..0af618287c9463 --- /dev/null +++ b/test/simple/test-http-client-reject-chunked-with-content-length.js @@ -0,0 +1,26 @@ +var common = require('../common'); +var http = require('http'); +var net = require('net'); +var assert = require('assert'); + +var reqstr = 'HTTP/1.1 200 OK\r\n' + + 'Content-Length: 1\r\n' + + 'Transfer-Encoding: chunked\r\n\r\n'; + +var server = net.createServer(function(socket) { + socket.write(reqstr); +}); + +server.listen(common.PORT, function() { + // The callback should not be called because the server is sending + // both a Content-Length header and a Transfer-Encoding: chunked + // header, which is a violation of the HTTP spec. + var req = http.get({port:common.PORT}, function(res) { + assert.fail(null, null, 'callback should not be called'); + }); + req.on('error', common.mustCall(function(err) { + assert(/^Parse Error/.test(err.message)); + assert.equal(err.code, 'HPE_UNEXPECTED_CONTENT_LENGTH'); + server.close(); + })); +}); diff --git a/test/simple/test-http-client-reject-cr-no-lf.js b/test/simple/test-http-client-reject-cr-no-lf.js new file mode 100644 index 00000000000000..cea441c277dbcf --- /dev/null +++ b/test/simple/test-http-client-reject-cr-no-lf.js @@ -0,0 +1,25 @@ +var common = require('../common'); +var http = require('http'); +var net = require('net'); +var assert = require('assert'); + +var reqstr = 'HTTP/1.1 200 OK\r\n' + + 'Foo: Bar\r' + + 'Content-Length: 1\r\n\r\n'; + +var server = net.createServer(function(socket) { + socket.write(reqstr); +}); + +server.listen(common.PORT, function() { + // The callback should not be called because the server is sending a + // header field that ends only in \r with no following \n + var req = http.get({port:common.PORT}, function(res) { + assert.fail(null, null, 'callback should not be called'); + }); + req.on('error', common.mustCall(function(err) { + assert(/^Parse Error/.test(err.message)); + assert.equal(err.code, 'HPE_LF_EXPECTED'); + server.close(); + })); +}); \ No newline at end of file diff --git a/test/simple/test-http-double-content-length.js b/test/simple/test-http-double-content-length.js new file mode 100644 index 00000000000000..fbe68956d33b4a --- /dev/null +++ b/test/simple/test-http-double-content-length.js @@ -0,0 +1,31 @@ +var common = require('../common'); +var http = require('http'); +var assert = require('assert'); + +// The callback should never be invoked because the server +// should respond with a 400 Client Error when a double +// Content-Length header is received. +var server = http.createServer(function(req, res) { + assert(false, 'callback should not have been invoked'); + res.end(); +}); +server.on('clientError', common.mustCall(function(err, socket) { + assert(/^Parse Error/.test(err.message)); + assert.equal(err.code, 'HPE_UNEXPECTED_CONTENT_LENGTH'); + socket.destroy(); +})); + +server.listen(common.PORT, function() { + var req = http.get({ + port: common.PORT, + // Send two content-length header values. + headers: {'Content-Length': [1, 2]}}, + function(res) { + assert.fail(null, null, 'an error should have occurred'); + server.close(); + } + ); + req.on('error', common.mustCall(function() { + server.close(); + })); +}); \ No newline at end of file diff --git a/test/simple/test-http-header-response-splitting.js b/test/simple/test-http-header-response-splitting.js index 1d3a85ce8b5624..bf9f8ee399f25b 100644 --- a/test/simple/test-http-header-response-splitting.js +++ b/test/simple/test-http-header-response-splitting.js @@ -18,6 +18,7 @@ // DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. +// Flags: --security-revert=CVE-2016-2216 var common = require('../common'), assert = require('assert'), diff --git a/test/simple/test-http-response-multi-content-length.js b/test/simple/test-http-response-multi-content-length.js new file mode 100644 index 00000000000000..180cc6ae560b04 --- /dev/null +++ b/test/simple/test-http-response-multi-content-length.js @@ -0,0 +1,50 @@ +var common = require('../common'); +var http = require('http'); +var assert = require('assert'); + +var MAX_COUNT = 2; + +var server = http.createServer(function(req, res) { + var num = req.headers['x-num']; + // TODO(@jasnell) At some point this should be refactored as the API + // should not be allowing users to set multiple content-length values + // in the first place. + switch (num) { + case '1': + res.setHeader('content-length', [2, 1]); + break; + case '2': + res.writeHead(200, {'content-length': [1, 2]}); + break; + default: + assert.fail(null, null, 'should never get here'); + } + res.end('ok'); +}); + +var count = 0; + +server.listen(common.PORT, common.mustCall(function() { + for (var n = 1; n <= MAX_COUNT ; n++) { + // This runs twice, the first time, the server will use + // setHeader, the second time it uses writeHead. In either + // case, the error handler must be called because the client + // is not allowed to accept multiple content-length headers. + http.get( + {port:common.PORT, headers:{'x-num': n}}, + function(res) { + assert(false, 'client allowed multiple content-length headers.'); + } + ).on('error', common.mustCall(function(err) { + assert(/^Parse Error/.test(err.message)); + assert.equal(err.code, 'HPE_UNEXPECTED_CONTENT_LENGTH'); + count++; + if (count === MAX_COUNT) + server.close(); + })); + } +})); + +process.on('exit', function() { + assert.equal(count, MAX_COUNT); +}); \ No newline at end of file diff --git a/test/simple/test-http-server-reject-chunked-with-content-length.js b/test/simple/test-http-server-reject-chunked-with-content-length.js new file mode 100644 index 00000000000000..1f2c34ea211e43 --- /dev/null +++ b/test/simple/test-http-server-reject-chunked-with-content-length.js @@ -0,0 +1,29 @@ +var common = require('../common'); +var http = require('http'); +var net = require('net'); +var assert = require('assert'); + +var reqstr = 'POST / HTTP/1.1\r\n' + + 'Content-Length: 1\r\n' + + 'Transfer-Encoding: chunked\r\n\r\n'; + +var server = http.createServer(function(req, res) { + assert.fail(null, null, 'callback should not be invoked'); +}); +server.on('clientError', common.mustCall(function(err) { + assert(/^Parse Error/.test(err.message)); + assert.equal(err.code, 'HPE_UNEXPECTED_CONTENT_LENGTH'); + server.close(); +})); +server.listen(common.PORT, function() { + var client = net.connect({port: common.PORT}, function() { + client.write(reqstr); + client.end(); + }); + client.on('data', function(data) { + // Should not get to this point because the server should simply + // close the connection without returning any data. + assert.fail(null, null, 'no data should be returned by the server'); + }); + client.on('end', common.mustCall(function() {})); +}); \ No newline at end of file diff --git a/test/simple/test-http-server-reject-cr-no-lf.js b/test/simple/test-http-server-reject-cr-no-lf.js new file mode 100644 index 00000000000000..76a5902924f538 --- /dev/null +++ b/test/simple/test-http-server-reject-cr-no-lf.js @@ -0,0 +1,28 @@ +var common = require('../common'); +var net = require('net'); +var http = require('http'); +var assert = require('assert'); + +var str = 'GET / HTTP/1.1\r\n' + + 'Dummy: Header\r' + + 'Content-Length: 1\r\n' + + '\r\n'; + +var server = http.createServer(function(req, res) { + assert.fail(null, null, 'this should not be called'); +}); +server.on('clientError', common.mustCall(function(err) { + assert(/^Parse Error/.test(err.message)); + assert.equal(err.code, 'HPE_LF_EXPECTED'); + server.close(); +})); +server.listen(common.PORT, function() { + var client = net.connect({port:common.PORT}, function() { + client.on('data', function(chunk) { + assert.fail(null, null, 'this should not be called'); + }); + client.on('end', common.mustCall(function() {})); + client.write(str); + client.end(); + }); +});