packages/nodejs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

!45 fix CVE-2020-8265 CVE-2020-8287

Browse Source 
From: @xinghe_1
Reviewed-by: @solarhu
Signed-off-by: @solarhu
This commit is contained in:
openeuler-ci-bot 2021-02-23 11:43:30 +08:00 committed by Gitee
commit 6c40c39f42
4 changed files with 402 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

164
CVE-2020-8265.patch Normal file
View File

@ -0,0 +1,164 @@
From 5b00de7d67a1372aa342115ad28edd3f78268bb6 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
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 <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
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<v8::Object> req_wrap_obj) {
StreamReq::ResetObject(req_wrap_obj);
}
+ BaseObjectPtr<AsyncWrap> 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<AsyncWrap> 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 <typename OtherBase>
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<Value>& 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<AsyncWrap> wrap_obj;
};
using JSMethodFunction = void(const v8::FunctionCallbackInfo<v8::Value>& 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());
+ });
+}));

79
CVE-2020-8287-1.patch Normal file
View File

@ -0,0 +1,79 @@
From 92d430917a63a567bb528100371263c46e50ee4a Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor@indutny.com>
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 <fedor.indutny@gmail.com>
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

151
CVE-2020-8287-2.patch Normal file
View File

@ -0,0 +1,151 @@
From 420244e4d9ca6de2612e7f503f5c87e448fbc14b Mon Sep 17 00:00:00 2001
From: Matteo Collina <hello@matteocollina.com>
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 <fedor.indutny@gmail.com>
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();
+ }));
+}));

9
nodejs.spec
View File

@ -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 <xinghe1@huawei.com> - 1:12.18.4-3
- fix CVE-2020-8265 CVE-2020-8287
* Mon Jan 04 2020 huanghaitao <huanghaitao8@huawei.com> - 1:12.18.4-2
- Make AARCH64 compile on 64KB physical pages to fix build error