diff --git a/CVE-2020-8265.patch b/CVE-2020-8265.patch new file mode 100644 index 0000000000000000000000000000000000000000..534550e3ec19ae3d97390a5af9e1dff98a20872b --- /dev/null +++ b/CVE-2020-8265.patch @@ -0,0 +1,164 @@ +From 5b00de7d67a1372aa342115ad28edd3f78268bb6 Mon Sep 17 00:00:00 2001 +From: James M Snell +Date: Thu, 12 Nov 2020 12:34:33 -0800 +Subject: [PATCH] src: retain pointers to WriteWrap/ShutdownWrap + +Avoids potential use-after-free when wrap req's are synchronously +destroyed. + +CVE-ID: CVE-2020-8265 +Fixes: https://github.com/nodejs-private/node-private/issues/227 +PR-URL: https://github.com/nodejs-private/node-private/pull/230 +Refs: https://hackerone.com/bugs?subject=nodejs&report_id=988103 +Reviewed-By: Anna Henningsen +Reviewed-By: Matteo Collina +Reviewed-By: Rich Trott +Reference: https://github.com/nodejs/node/commit/5b00de7d67a1372aa342115ad28edd3f78268bb6 +--- + src/stream_base-inl.h | 11 +++- + src/stream_base.cc | 2 +- + src/stream_base.h | 1 + + .../test-tls-use-after-free-regression.js | 58 +++++++++++++++++++ + 4 files changed, 68 insertions(+), 4 deletions(-) + create mode 100644 test/parallel/test-tls-use-after-free-regression.js + +diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h +index dd80683af10..1603a2fb2e0 100644 +--- a/src/stream_base-inl.h ++++ b/src/stream_base-inl.h +@@ -163,8 +163,11 @@ inline int StreamBase::Shutdown(v8::Local req_wrap_obj) { + StreamReq::ResetObject(req_wrap_obj); + } + ++ BaseObjectPtr req_wrap_ptr; + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(GetAsyncWrap()); + ShutdownWrap* req_wrap = CreateShutdownWrap(req_wrap_obj); ++ if (req_wrap != nullptr) ++ req_wrap_ptr.reset(req_wrap->GetAsyncWrap()); + int err = DoShutdown(req_wrap); + + if (err != 0 && req_wrap != nullptr) { +@@ -198,7 +201,7 @@ inline StreamWriteResult StreamBase::Write( + if (send_handle == nullptr) { + err = DoTryWrite(&bufs, &count); + if (err != 0 || count == 0) { +- return StreamWriteResult { false, err, nullptr, total_bytes }; ++ return StreamWriteResult { false, err, nullptr, total_bytes, {} }; + } + } + +@@ -208,13 +211,14 @@ inline StreamWriteResult StreamBase::Write( + if (!env->write_wrap_template() + ->NewInstance(env->context()) + .ToLocal(&req_wrap_obj)) { +- return StreamWriteResult { false, UV_EBUSY, nullptr, 0 }; ++ return StreamWriteResult { false, UV_EBUSY, nullptr, 0, {} }; + } + StreamReq::ResetObject(req_wrap_obj); + } + + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(GetAsyncWrap()); + WriteWrap* req_wrap = CreateWriteWrap(req_wrap_obj); ++ BaseObjectPtr req_wrap_ptr(req_wrap->GetAsyncWrap()); + + err = DoWrite(req_wrap, bufs, count, send_handle); + bool async = err == 0; +@@ -232,7 +236,8 @@ inline StreamWriteResult StreamBase::Write( + ClearError(); + } + +- return StreamWriteResult { async, err, req_wrap, total_bytes }; ++ return StreamWriteResult { ++ async, err, req_wrap, total_bytes, std::move(req_wrap_ptr) }; + } + + template +diff --git a/src/stream_base.cc b/src/stream_base.cc +index 516f57e40bf..06032e2c096 100644 +--- a/src/stream_base.cc ++++ b/src/stream_base.cc +@@ -259,7 +259,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { + + // Immediate failure or success + if (err != 0 || count == 0) { +- SetWriteResult(StreamWriteResult { false, err, nullptr, data_size }); ++ SetWriteResult(StreamWriteResult { false, err, nullptr, data_size, {} }); + return err; + } + +diff --git a/src/stream_base.h b/src/stream_base.h +index eb75fdc8339..fafd327d75d 100644 +--- a/src/stream_base.h ++++ b/src/stream_base.h +@@ -24,6 +24,7 @@ struct StreamWriteResult { + int err; + WriteWrap* wrap; + size_t bytes; ++ BaseObjectPtr wrap_obj; + }; + + using JSMethodFunction = void(const v8::FunctionCallbackInfo& args); +diff --git a/test/parallel/test-tls-use-after-free-regression.js b/test/parallel/test-tls-use-after-free-regression.js +new file mode 100644 +index 00000000000..51835fc0339 +--- /dev/null ++++ b/test/parallel/test-tls-use-after-free-regression.js +@@ -0,0 +1,58 @@ ++'use strict'; ++ ++const common = require('../common'); ++ ++if (!common.hasCrypto) ++ common.skip('missing crypto'); ++ ++const https = require('https'); ++const tls = require('tls'); ++ ++const kMessage = ++ 'GET / HTTP/1.1\r\nHost: localhost\r\nConnection: Keep-alive\r\n\r\n'; ++ ++const key = `-----BEGIN EC PARAMETERS----- ++BggqhkjOPQMBBw== ++-----END EC PARAMETERS----- ++-----BEGIN EC PRIVATE KEY----- ++MHcCAQEEIDKfHHbiJMdu2STyHL11fWC7psMY19/gUNpsUpkwgGACoAoGCCqGSM49 ++AwEHoUQDQgAEItqm+pYj3Ca8bi5mBs+H8xSMxuW2JNn4I+kw3aREsetLk8pn3o81 ++PWBiTdSZrGBGQSy+UAlQvYeE6Z/QXQk8aw== ++-----END EC PRIVATE KEY-----`; ++ ++const cert = `-----BEGIN CERTIFICATE----- ++MIIBhjCCASsCFDJU1tCo88NYU//pE+DQKO9hUDsFMAoGCCqGSM49BAMCMEUxCzAJ ++BgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5l ++dCBXaWRnaXRzIFB0eSBMdGQwHhcNMjAwOTIyMDg1NDU5WhcNNDgwMjA3MDg1NDU5 ++WjBFMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwY ++SW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcD ++QgAEItqm+pYj3Ca8bi5mBs+H8xSMxuW2JNn4I+kw3aREsetLk8pn3o81PWBiTdSZ ++rGBGQSy+UAlQvYeE6Z/QXQk8azAKBggqhkjOPQQDAgNJADBGAiEA7Bdn4F87KqIe ++Y/ABy/XIXXpFUb2nyv3zV7POQi2lPcECIQC3UWLmfiedpiIKsf9YRIyO0uEood7+ ++glj2R1NNr1X68w== ++-----END CERTIFICATE-----`; ++ ++const server = https.createServer( ++ { key, cert }, ++ common.mustCall((req, res) => { ++ res.writeHead(200); ++ res.end('boom goes the dynamite\n'); ++ }, 3)); ++ ++server.listen(0, common.mustCall(() => { ++ const socket = ++ tls.connect( ++ server.address().port, ++ 'localhost', ++ { rejectUnauthorized: false }, ++ common.mustCall(() => { ++ socket.write(kMessage); ++ socket.write(kMessage); ++ socket.write(kMessage); ++ })); ++ ++ socket.on('data', common.mustCall(() => socket.destroy())); ++ socket.on('close', () => { ++ setImmediate(() => server.close()); ++ }); ++})); diff --git a/CVE-2020-8287-1.patch b/CVE-2020-8287-1.patch new file mode 100644 index 0000000000000000000000000000000000000000..3761c94220efdbe4357ff1f5fe0cc83b49eb07ee --- /dev/null +++ b/CVE-2020-8287-1.patch @@ -0,0 +1,79 @@ +From 92d430917a63a567bb528100371263c46e50ee4a Mon Sep 17 00:00:00 2001 +From: Fedor Indutny +Date: Wed, 18 Nov 2020 20:50:21 -0800 +Subject: [PATCH] http: unset `F_CHUNKED` on new `Transfer-Encoding` + +Duplicate `Transfer-Encoding` header should be a treated as a single, +but with original header values concatenated with a comma separator. In +the light of this, even if the past `Transfer-Encoding` ended with +`chunked`, we should be not let the `F_CHUNKED` to leak into the next +header, because mere presence of another header indicates that `chunked` +is not the last transfer-encoding token. + +CVE-ID: CVE-2020-8287 +PR-URL: https://github.com/nodejs-private/node-private/pull/236 +Reviewed-By: Fedor Indutny +Reference: https://github.com/nodejs/node/commit/92d430917a63a567bb528100371263c46e50ee4a +--- + deps/http_parser/http_parser.c | 7 +++++++ + deps/http_parser/test.c | 26 ++++++++++++++++++++++++++ + 2 files changed, 33 insertions(+) + +diff --git a/deps/http_parser/http_parser.c b/deps/http_parser/http_parser.c +index 0f76b6a..5cc951a 100644 +--- a/deps/http_parser/http_parser.c ++++ b/deps/http_parser/http_parser.c +@@ -1339,6 +1339,13 @@ reexecute: + } else if (parser->index == sizeof(TRANSFER_ENCODING)-2) { + parser->header_state = h_transfer_encoding; + parser->flags |= F_TRANSFER_ENCODING; ++ ++ /* Multiple `Transfer-Encoding` headers should be treated as ++ * one, but with values separate by a comma. ++ * ++ * See: https://tools.ietf.org/html/rfc7230#section-3.2.2 ++ */ ++ parser->flags &= ~F_CHUNKED; + } + break; + +diff --git a/deps/http_parser/test.c b/deps/http_parser/test.c +index c979467..f185c56 100644 +--- a/deps/http_parser/test.c ++++ b/deps/http_parser/test.c +@@ -2045,6 +2045,32 @@ const struct message responses[] = + ,.body= "2\r\nOK\r\n0\r\n\r\n" + ,.num_chunks_complete= 0 + } ++#define HTTP_200_DUPLICATE_TE_NOT_LAST_CHUNKED 30 ++, {.name= "HTTP 200 response with `chunked` and duplicate Transfer-Encoding" ++ ,.type= HTTP_RESPONSE ++ ,.raw= "HTTP/1.1 200 OK\r\n" ++ "Transfer-Encoding: chunked\r\n" ++ "Transfer-Encoding: identity\r\n" ++ "\r\n" ++ "2\r\n" ++ "OK\r\n" ++ "0\r\n" ++ "\r\n" ++ ,.should_keep_alive= FALSE ++ ,.message_complete_on_eof= TRUE ++ ,.http_major= 1 ++ ,.http_minor= 1 ++ ,.status_code= 200 ++ ,.response_status= "OK" ++ ,.content_length= -1 ++ ,.num_headers= 2 ++ ,.headers= ++ { { "Transfer-Encoding", "chunked" } ++ , { "Transfer-Encoding", "identity" } ++ } ++ ,.body= "2\r\nOK\r\n0\r\n\r\n" ++ ,.num_chunks_complete= 0 ++ } + }; + + /* strnlen() is a POSIX.2008 addition. Can't rely on it being available so +-- +2.23.0 + diff --git a/CVE-2020-8287-2.patch b/CVE-2020-8287-2.patch new file mode 100644 index 0000000000000000000000000000000000000000..e88984e2414c599c7ead781e9ef30bb6d89aabef --- /dev/null +++ b/CVE-2020-8287-2.patch @@ -0,0 +1,151 @@ +From 420244e4d9ca6de2612e7f503f5c87e448fbc14b Mon Sep 17 00:00:00 2001 +From: Matteo Collina +Date: Thu, 22 Oct 2020 14:10:51 +0200 +Subject: [PATCH] http: unset `F_CHUNKED` on new `Transfer-Encoding` + +Duplicate `Transfer-Encoding` header should be a treated as a single, +but with original header values concatenated with a comma separator. In +the light of this, even if the past `Transfer-Encoding` ended with +`chunked`, we should be not let the `F_CHUNKED` to leak into the next +header, because mere presence of another header indicates that `chunked` +is not the last transfer-encoding token. + +Ref: https://github.com/nodejs-private/llhttp-private/pull/3 +See: https://hackerone.com/bugs?report_id=1002188&subject=nodejs + +CVE-ID: CVE-2020-8287 +PR-URL: https://github.com/nodejs-private/node-private/pull/236 +Reviewed-By: Fedor Indutny +Reference: https://github.com/nodejs/node/commit/420244e4d9ca6de2612e7f503f5c87e448fbc14b +--- + deps/llhttp/src/llhttp.c | 36 ++++++++++++++- + .../test-http-transfer-encoding-smuggling.js | 46 +++++++++++++++++++ + 2 files changed, 80 insertions(+), 2 deletions(-) + create mode 100644 test/parallel/test-http-transfer-encoding-smuggling.js + +diff --git a/deps/llhttp/src/llhttp.c b/deps/llhttp/src/llhttp.c +index acc35479f88..3019c410963 100644 +--- a/deps/llhttp/src/llhttp.c ++++ b/deps/llhttp/src/llhttp.c +@@ -813,6 +813,14 @@ int llhttp__internal__c_or_flags_16( + return 0; + } + ++int llhttp__internal__c_and_flags( ++ llhttp__internal_t* state, ++ const unsigned char* p, ++ const unsigned char* endp) { ++ state->flags &= -9; ++ return 0; ++} ++ + int llhttp__internal__c_update_header_state_7( + llhttp__internal_t* state, + const unsigned char* p, +@@ -5974,10 +5982,18 @@ static llparse_state_t llhttp__internal__run( + /* UNREACHABLE */; + abort(); + } ++ s_n_llhttp__internal__n_invoke_and_flags: { ++ switch (llhttp__internal__c_and_flags(state, p, endp)) { ++ default: ++ goto s_n_llhttp__internal__n_header_value_te_chunked; ++ } ++ /* UNREACHABLE */; ++ abort(); ++ } + s_n_llhttp__internal__n_invoke_or_flags_16: { + switch (llhttp__internal__c_or_flags_16(state, p, endp)) { + default: +- goto s_n_llhttp__internal__n_header_value_te_chunked; ++ goto s_n_llhttp__internal__n_invoke_and_flags; + } + /* UNREACHABLE */; + abort(); +@@ -7625,6 +7641,14 @@ int llhttp__internal__c_or_flags_16( + return 0; + } + ++int llhttp__internal__c_and_flags( ++ llhttp__internal_t* state, ++ const unsigned char* p, ++ const unsigned char* endp) { ++ state->flags &= -9; ++ return 0; ++} ++ + int llhttp__internal__c_update_header_state_7( + llhttp__internal_t* state, + const unsigned char* p, +@@ -12522,10 +12546,18 @@ static llparse_state_t llhttp__internal__run( + /* UNREACHABLE */; + abort(); + } ++ s_n_llhttp__internal__n_invoke_and_flags: { ++ switch (llhttp__internal__c_and_flags(state, p, endp)) { ++ default: ++ goto s_n_llhttp__internal__n_header_value_te_chunked; ++ } ++ /* UNREACHABLE */; ++ abort(); ++ } + s_n_llhttp__internal__n_invoke_or_flags_16: { + switch (llhttp__internal__c_or_flags_16(state, p, endp)) { + default: +- goto s_n_llhttp__internal__n_header_value_te_chunked; ++ goto s_n_llhttp__internal__n_invoke_and_flags; + } + /* UNREACHABLE */; + abort(); +diff --git a/test/parallel/test-http-transfer-encoding-smuggling.js b/test/parallel/test-http-transfer-encoding-smuggling.js +new file mode 100644 +index 00000000000..9d97db4c0a2 +--- /dev/null ++++ b/test/parallel/test-http-transfer-encoding-smuggling.js +@@ -0,0 +1,46 @@ ++'use strict'; ++ ++const common = require('../common'); ++ ++const assert = require('assert'); ++const http = require('http'); ++const net = require('net'); ++ ++const msg = [ ++ 'POST / HTTP/1.1', ++ 'Host: 127.0.0.1', ++ 'Transfer-Encoding: chunked', ++ 'Transfer-Encoding: chunked-false', ++ 'Connection: upgrade', ++ '', ++ '1', ++ 'A', ++ '0', ++ '', ++ 'GET /flag HTTP/1.1', ++ 'Host: 127.0.0.1', ++ '', ++ '', ++].join('\r\n'); ++ ++// Verify that the server is called only once even with a smuggled request. ++ ++const server = http.createServer(common.mustCall((req, res) => { ++ res.end(); ++}, 1)); ++ ++function send(next) { ++ const client = net.connect(server.address().port, 'localhost'); ++ client.setEncoding('utf8'); ++ client.on('error', common.mustNotCall()); ++ client.on('end', next); ++ client.write(msg); ++ client.resume(); ++} ++ ++server.listen(0, common.mustCall((err) => { ++ assert.ifError(err); ++ send(common.mustCall(() => { ++ server.close(); ++ })); ++})); diff --git a/nodejs.spec b/nodejs.spec index 2f2105272a6414a4cfeff3398c022e29c4ac6732..2e1922a7cc928de7373a006a2a7b32bd7a00d173 100644 --- a/nodejs.spec +++ b/nodejs.spec @@ -1,5 +1,5 @@ %bcond_with bootstrap -%global baserelease 2 +%global baserelease 3 %{?!_pkgdocdir:%global _pkgdocdir %{_docdir}/%{name}-%{version}} %global nodejs_epoch 1 %global nodejs_major 12 @@ -85,6 +85,10 @@ Patch0002: 0002-Install-both-binaries-and-use-libdir.patch Patch0003: 0003-Modify-openEuler-aarch64-v8_os_page_size-to-64.patch %endif Patch0004: 0004-Make-AARCH64-compile-on-64KB-physical-pages.patch +Patch0005: CVE-2020-8265.patch +Patch0006: CVE-2020-8287-1.patch +Patch0007: CVE-2020-8287-2.patch + BuildRequires: python3-devel BuildRequires: zlib-devel BuildRequires: brotli-devel @@ -486,6 +490,9 @@ end %{_pkgdocdir}/npm/docs %changelog +* Tue Feb 09 2021 xinghe - 1:12.18.4-3 +- fix CVE-2020-8265 CVE-2020-8287 + * Mon Jan 04 2020 huanghaitao - 1:12.18.4-2 - Make AARCH64 compile on 64KB physical pages to fix build error