Fix CVE-2024-22019 BZ#2265709 Instead of updating llhttp to version 6.1.0 (which would be too big jump from the currect version), Richard bacported the missing feature into llhttp, so we can use pretty much the same fix in the nodejs itsef, as was originally used for upstream (v18): https://github.com/nodejs/node/commit/03a5c34a829742f1c47b68f831b2940af44addf6 From 59a3461c075ac2d6d44bd90a291db0b8ed688af8 Mon Sep 17 00:00:00 2001 From: Richard Lau Date: Fri, 1 Mar 2024 17:51:39 +0000 Subject: [PATCH 1/2] deps: port llhttp ef539d7293fc28577d45f4d7eb6ff5aabb59b372 Port https://github.com/nodejs/llhttp/pull/264 to llhttp 2.1.x. --- deps/llhttp/include/llhttp.h | 3 + deps/llhttp/src/api.c | 22 +++++++ deps/llhttp/src/llhttp.c | 122 +++++++++++++++++++++++++++++++---- 3 files changed, 133 insertions(+), 14 deletions(-) diff --git a/deps/llhttp/include/llhttp.h b/deps/llhttp/include/llhttp.h index fe3a927fc6..d56e5b3d20 100644 --- a/deps/llhttp/include/llhttp.h +++ b/deps/llhttp/include/llhttp.h @@ -255,6 +255,9 @@ struct llhttp_settings_s { */ llhttp_cb on_headers_complete; + /* Possible return values 0, -1, HPE_USER */ + llhttp_data_cb on_chunk_parameters; + llhttp_data_cb on_body; /* Possible return values 0, -1, `HPE_PAUSED` */ diff --git a/deps/llhttp/src/api.c b/deps/llhttp/src/api.c index 6f7246546d..1b7ad0e65b 100644 --- a/deps/llhttp/src/api.c +++ b/deps/llhttp/src/api.c @@ -15,6 +15,21 @@ err = settings->NAME(__VA_ARGS__); \ } while (0) +#define SPAN_CALLBACK_MAYBE(PARSER, NAME, START, LEN) \ + do { \ + const llhttp_settings_t* settings; \ + settings = (const llhttp_settings_t*) (PARSER)->settings; \ + if (settings == NULL || settings->NAME == NULL) { \ + err = 0; \ + break; \ + } \ + err = settings->NAME((PARSER), (START), (LEN)); \ + if (err == -1) { \ + err = HPE_USER; \ + llhttp_set_error_reason((PARSER), "Span callback error in " #NAME); \ + } \ + } while (0) + void llhttp_init(llhttp_t* parser, llhttp_type_t type, const llhttp_settings_t* settings) { llhttp__internal_init(parser); @@ -202,6 +217,13 @@ int llhttp__on_chunk_header(llhttp_t* s, const char* p, const char* endp) { } +int llhttp__on_chunk_parameters(llhttp_t* s, const char* p, const char* endp) { + int err; + SPAN_CALLBACK_MAYBE(s, on_chunk_parameters, p, endp - p); + return err; +} + + int llhttp__on_chunk_complete(llhttp_t* s, const char* p, const char* endp) { int err; CALLBACK_MAYBE(s, on_chunk_complete, s); diff --git a/deps/llhttp/src/llhttp.c b/deps/llhttp/src/llhttp.c index f5439cbf3e..6281913d10 100644 --- a/deps/llhttp/src/llhttp.c +++ b/deps/llhttp/src/llhttp.c @@ -307,6 +307,8 @@ enum llparse_state_e { s_n_llhttp__internal__n_invoke_is_equal_content_length, s_n_llhttp__internal__n_chunk_size_almost_done, s_n_llhttp__internal__n_chunk_parameters, + s_n_llhttp__internal__n_span_start_llhttp__on_chunk_parameters, + s_n_llhttp__internal__n_chunk_parameters_ows, s_n_llhttp__internal__n_chunk_size_otherwise, s_n_llhttp__internal__n_chunk_size, s_n_llhttp__internal__n_chunk_size_digit, @@ -482,6 +484,10 @@ int llhttp__on_body( llhttp__internal_t* s, const unsigned char* p, const unsigned char* endp); +int llhttp__on_chunk_parameters( + llhttp__internal_t* s, const unsigned char* p, + const unsigned char* endp); + int llhttp__on_status( llhttp__internal_t* s, const unsigned char* p, const unsigned char* endp); @@ -1118,8 +1124,7 @@ static llparse_state_t llhttp__internal__run( goto s_n_llhttp__internal__n_chunk_parameters; } case 2: { - p++; - goto s_n_llhttp__internal__n_chunk_size_almost_done; + goto s_n_llhttp__internal__n_span_end_llhttp__on_chunk_parameters; } default: { goto s_n_llhttp__internal__n_error_10; @@ -1128,6 +1133,34 @@ static llparse_state_t llhttp__internal__run( /* UNREACHABLE */; abort(); } + case s_n_llhttp__internal__n_span_start_llhttp__on_chunk_parameters: + s_n_llhttp__internal__n_span_start_llhttp__on_chunk_parameters: { + if (p == endp) { + return s_n_llhttp__internal__n_span_start_llhttp__on_chunk_parameters; + } + state->_span_pos0 = (void*) p; + state->_span_cb0 = llhttp__on_chunk_parameters; + goto s_n_llhttp__internal__n_chunk_parameters; + /* UNREACHABLE */; + abort(); + } + case s_n_llhttp__internal__n_chunk_parameters_ows: + s_n_llhttp__internal__n_chunk_parameters_ows: { + if (p == endp) { + return s_n_llhttp__internal__n_chunk_parameters_ows; + } + switch (*p) { + case ' ': { + p++; + goto s_n_llhttp__internal__n_chunk_parameters_ows; + } + default: { + goto s_n_llhttp__internal__n_span_start_llhttp__on_chunk_parameters; + } + } + /* UNREACHABLE */; + abort(); + } case s_n_llhttp__internal__n_chunk_size_otherwise: s_n_llhttp__internal__n_chunk_size_otherwise: { if (p == endp) { @@ -1138,13 +1171,9 @@ static llparse_state_t llhttp__internal__run( p++; goto s_n_llhttp__internal__n_chunk_size_almost_done; } - case ' ': { - p++; - goto s_n_llhttp__internal__n_chunk_parameters; - } case ';': { p++; - goto s_n_llhttp__internal__n_chunk_parameters; + goto s_n_llhttp__internal__n_chunk_parameters_ows; } default: { goto s_n_llhttp__internal__n_error_11; @@ -5449,6 +5478,24 @@ static llparse_state_t llhttp__internal__run( /* UNREACHABLE */; abort(); } + s_n_llhttp__internal__n_span_end_llhttp__on_chunk_parameters: { + const unsigned char* start; + int err; + + start = state->_span_pos0; + state->_span_pos0 = NULL; + err = llhttp__on_chunk_parameters(state, start, p); + if (err != 0) { + state->error = err; + state->error_pos = (const char*) (p + 1); + state->_current = (void*) (intptr_t) s_n_llhttp__internal__n_chunk_size_almost_done; + return s_error; + } + p++; + goto s_n_llhttp__internal__n_chunk_size_almost_done; + /* UNREACHABLE */; + abort(); + } s_n_llhttp__internal__n_error_10: { state->error = 0x2; state->reason = "Invalid character in chunk parameters"; @@ -7405,6 +7452,8 @@ enum llparse_state_e { s_n_llhttp__internal__n_invoke_is_equal_content_length, s_n_llhttp__internal__n_chunk_size_almost_done, s_n_llhttp__internal__n_chunk_parameters, + s_n_llhttp__internal__n_span_start_llhttp__on_chunk_parameters, + s_n_llhttp__internal__n_chunk_parameters_ows, s_n_llhttp__internal__n_chunk_size_otherwise, s_n_llhttp__internal__n_chunk_size, s_n_llhttp__internal__n_chunk_size_digit, @@ -7575,6 +7624,10 @@ int llhttp__on_body( llhttp__internal_t* s, const unsigned char* p, const unsigned char* endp); +int llhttp__on_chunk_parameters( + llhttp__internal_t* s, const unsigned char* p, + const unsigned char* endp); + int llhttp__on_status( llhttp__internal_t* s, const unsigned char* p, const unsigned char* endp); @@ -8176,8 +8229,7 @@ static llparse_state_t llhttp__internal__run( goto s_n_llhttp__internal__n_chunk_parameters; } case 2: { - p++; - goto s_n_llhttp__internal__n_chunk_size_almost_done; + goto s_n_llhttp__internal__n_span_end_llhttp__on_chunk_parameters; } default: { goto s_n_llhttp__internal__n_error_6; @@ -8186,6 +8238,34 @@ static llparse_state_t llhttp__internal__run( /* UNREACHABLE */; abort(); } + case s_n_llhttp__internal__n_span_start_llhttp__on_chunk_parameters: + s_n_llhttp__internal__n_span_start_llhttp__on_chunk_parameters: { + if (p == endp) { + return s_n_llhttp__internal__n_span_start_llhttp__on_chunk_parameters; + } + state->_span_pos0 = (void*) p; + state->_span_cb0 = llhttp__on_chunk_parameters; + goto s_n_llhttp__internal__n_chunk_parameters; + /* UNREACHABLE */; + abort(); + } + case s_n_llhttp__internal__n_chunk_parameters_ows: + s_n_llhttp__internal__n_chunk_parameters_ows: { + if (p == endp) { + return s_n_llhttp__internal__n_chunk_parameters_ows; + } + switch (*p) { + case ' ': { + p++; + goto s_n_llhttp__internal__n_chunk_parameters_ows; + } + default: { + goto s_n_llhttp__internal__n_span_start_llhttp__on_chunk_parameters; + } + } + /* UNREACHABLE */; + abort(); + } case s_n_llhttp__internal__n_chunk_size_otherwise: s_n_llhttp__internal__n_chunk_size_otherwise: { if (p == endp) { @@ -8196,13 +8276,9 @@ static llparse_state_t llhttp__internal__run( p++; goto s_n_llhttp__internal__n_chunk_size_almost_done; } - case ' ': { - p++; - goto s_n_llhttp__internal__n_chunk_parameters; - } case ';': { p++; - goto s_n_llhttp__internal__n_chunk_parameters; + goto s_n_llhttp__internal__n_chunk_parameters_ows; } default: { goto s_n_llhttp__internal__n_error_7; @@ -12296,6 +12372,24 @@ static llparse_state_t llhttp__internal__run( /* UNREACHABLE */; abort(); } + s_n_llhttp__internal__n_span_end_llhttp__on_chunk_parameters: { + const unsigned char* start; + int err; + + start = state->_span_pos0; + state->_span_pos0 = NULL; + err = llhttp__on_chunk_parameters(state, start, p); + if (err != 0) { + state->error = err; + state->error_pos = (const char*) (p + 1); + state->_current = (void*) (intptr_t) s_n_llhttp__internal__n_chunk_size_almost_done; + return s_error; + } + p++; + goto s_n_llhttp__internal__n_chunk_size_almost_done; + /* UNREACHABLE */; + abort(); + } s_n_llhttp__internal__n_error_6: { state->error = 0x2; state->reason = "Invalid character in chunk parameters"; -- 2.41.0 From f701cde064a9d375ea7dc531e3d61335e6c9e074 Mon Sep 17 00:00:00 2001 From: Paolo Insogna Date: Tue, 9 Jan 2024 18:10:04 +0100 Subject: [PATCH 2/2] http: add maximum chunk extension size PR-URL: https://github.com/nodejs-private/node-private/pull/520 Refs: https://github.com/nodejs-private/node-private/pull/518 CVE-ID: CVE-2024-22019 --- doc/api/errors.md | 12 ++ lib/_http_server.js | 7 + src/node_http_parser.cc | 20 ++- .../test-http-chunk-extensions-limit.js | 131 ++++++++++++++++++ 4 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http-chunk-extensions-limit.js diff --git a/doc/api/errors.md b/doc/api/errors.md index 0fc8cfcf19..0e4ec4a9e5 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2318,6 +2318,18 @@ malconfigured clients, if more than 8KB of HTTP header data is received then HTTP parsing will abort without a request or response object being created, and an `Error` with this code will be emitted. + + +### `HPE_CHUNK_EXTENSIONS_OVERFLOW` + + + +Too much data was received for a chunk extensions. In order to protect against +malicious or malconfigured clients, if more than 16 KiB of data is received +then an `Error` with this code will be emitted. + ### `HPE_UNEXPECTED_CONTENT_LENGTH` diff --git a/lib/_http_server.js b/lib/_http_server.js index 6d587d9d04..ae3b45fcba 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -636,6 +636,10 @@ const requestHeaderFieldsTooLargeResponse = Buffer.from( `HTTP/1.1 431 ${STATUS_CODES[431]}${CRLF}` + `Connection: close${CRLF}${CRLF}`, 'ascii' ); +const requestChunkExtensionsTooLargeResponse = Buffer.from( + `HTTP/1.1 413 ${STATUS_CODES[413]}\r\n` + + 'Connection: close\r\n\r\n', 'ascii', +); function socketOnError(e) { // Ignore further errors this.removeListener('error', socketOnError); @@ -649,6 +653,9 @@ function socketOnError(e) { case 'HPE_HEADER_OVERFLOW': response = requestHeaderFieldsTooLargeResponse; break; + case 'HPE_CHUNK_EXTENSIONS_OVERFLOW': + response = requestChunkExtensionsTooLargeResponse; + break; case 'ERR_HTTP_REQUEST_TIMEOUT': response = requestTimeoutResponse; break; diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index d3184bb1a1..dd81982d51 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -78,6 +78,8 @@ const uint32_t kOnExecute = 5; const uint32_t kOnTimeout = 6; // Any more fields than this will be flushed into JS const size_t kMaxHeaderFieldsCount = 32; +// Maximum size of chunk extensions +const size_t kMaxChunkExtensionsSize = 16384; inline bool IsOWS(char c) { return c == ' ' || c == '\t'; @@ -202,6 +204,7 @@ class Parser : public AsyncWrap, public StreamListener { int on_message_begin() { num_fields_ = num_values_ = 0; + chunk_extensions_nread_ = 0; url_.Reset(); status_message_.Reset(); header_parsing_start_time_ = uv_hrtime(); @@ -451,9 +454,22 @@ class Parser : public AsyncWrap, public StreamListener { return 0; } - // Reset nread for the next chunk + int on_chunk_extension(const char* at, size_t length) { + chunk_extensions_nread_ += length; + + if (chunk_extensions_nread_ > kMaxChunkExtensionsSize) { + llhttp_set_error_reason(&parser_, + "HPE_CHUNK_EXTENSIONS_OVERFLOW:Chunk extensions overflow"); + return HPE_USER; + } + + return 0; + } + + // Reset nread for the next chunk and also reset the extensions counter int on_chunk_header() { header_nread_ = 0; + chunk_extensions_nread_ = 0; return 0; } @@ -904,6 +920,7 @@ class Parser : public AsyncWrap, public StreamListener { unsigned int execute_depth_ = 0; bool pending_pause_ = false; uint64_t header_nread_ = 0; + uint64_t chunk_extensions_nread_ = 0; uint64_t max_http_header_size_; uint64_t headers_timeout_; uint64_t header_parsing_start_time_ = 0; @@ -938,6 +955,7 @@ const llhttp_settings_t Parser::settings = { Proxy::Raw, Proxy::Raw, Proxy::Raw, + Proxy::Raw, Proxy::Raw, Proxy::Raw, Proxy::Raw, diff --git a/test/parallel/test-http-chunk-extensions-limit.js b/test/parallel/test-http-chunk-extensions-limit.js new file mode 100644 index 0000000000..6868b3da6c --- /dev/null +++ b/test/parallel/test-http-chunk-extensions-limit.js @@ -0,0 +1,131 @@ +'use strict'; + +const common = require('../common'); +const http = require('http'); +const net = require('net'); +const assert = require('assert'); + +// Verify that chunk extensions are limited in size when sent all together. +{ + const server = http.createServer((req, res) => { + req.on('end', () => { + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end('bye'); + }); + + req.resume(); + }); + + server.listen(0, () => { + const sock = net.connect(server.address().port); + let data = ''; + + sock.on('data', (chunk) => data += chunk.toString('utf-8')); + + sock.on('end', common.mustCall(function() { + assert.strictEqual(data, 'HTTP/1.1 413 Payload Too Large\r\nConnection: close\r\n\r\n'); + server.close(); + })); + + sock.end('' + + 'GET / HTTP/1.1\r\n' + + 'Host: localhost:8080\r\n' + + 'Transfer-Encoding: chunked\r\n\r\n' + + '2;' + 'A'.repeat(20000) + '=bar\r\nAA\r\n' + + '0\r\n\r\n' + ); + }); +} + +// Verify that chunk extensions are limited in size when sent in intervals. +{ + const server = http.createServer((req, res) => { + req.on('end', () => { + res.writeHead(200, { 'Content-Type': 'text/plain' }); + res.end('bye'); + }); + + req.resume(); + }); + + server.listen(0, () => { + const sock = net.connect(server.address().port); + let remaining = 20000; + let data = ''; + + const interval = setInterval( + () => { + if (remaining > 0) { + sock.write('A'.repeat(1000)); + } else { + sock.write('=bar\r\nAA\r\n0\r\n\r\n'); + clearInterval(interval); + } + + remaining -= 1000; + }, + common.platformTimeout(20), + ).unref(); + + sock.on('data', (chunk) => data += chunk.toString('utf-8')); + + sock.on('end', common.mustCall(function() { + assert.strictEqual(data, 'HTTP/1.1 413 Payload Too Large\r\nConnection: close\r\n\r\n'); + server.close(); + })); + + sock.write('' + + 'GET / HTTP/1.1\r\n' + + 'Host: localhost:8080\r\n' + + 'Transfer-Encoding: chunked\r\n\r\n' + + '2;' + ); + }); +} + +// Verify the chunk extensions is correctly reset after a chunk +{ + const server = http.createServer((req, res) => { + req.on('end', () => { + res.writeHead(200, { 'content-type': 'text/plain', 'connection': 'close', 'date': 'now' }); + res.end('bye'); + }); + + req.resume(); + }); + + server.listen(0, () => { + const sock = net.connect(server.address().port); + let data = ''; + + sock.on('data', (chunk) => data += chunk.toString('utf-8')); + + sock.on('end', common.mustCall(function() { + assert.strictEqual( + data, + 'HTTP/1.1 200 OK\r\n' + + 'content-type: text/plain\r\n' + + 'connection: close\r\n' + + 'date: now\r\n' + + 'Transfer-Encoding: chunked\r\n' + + '\r\n' + + '3\r\n' + + 'bye\r\n' + + '0\r\n' + + '\r\n', + ); + + server.close(); + })); + + sock.end('' + + 'GET / HTTP/1.1\r\n' + + 'Host: localhost:8080\r\n' + + 'Transfer-Encoding: chunked\r\n\r\n' + + '2;' + 'A'.repeat(10000) + '=bar\r\nAA\r\n' + + '2;' + 'A'.repeat(10000) + '=bar\r\nAA\r\n' + + '2;' + 'A'.repeat(10000) + '=bar\r\nAA\r\n' + + '0\r\n\r\n' + ); + }); +} -- 2.41.0