diff --git a/CVE-2020-8174.patch b/CVE-2020-8174.patch new file mode 100644 index 0000000000000000000000000000000000000000..783df3890d3073c21abc5265b9a1d8103a6005e5 --- /dev/null +++ b/CVE-2020-8174.patch @@ -0,0 +1,134 @@ +From 656260b4b65fec3b10f6da3fdc9f11fb941aafb5 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= +Date: Thu, 4 Feb 2021 10:30:04 +0800 +Subject: [PATCH] napi: fix memory corruption vulnerability + +Fixes: https://hackerone.com/reports/784186 +CVE-ID: CVE-2020-8174 +PR-URL: https://github.com/nodejs-private/node-private/pull/195 +Reviewed-By: Anna Henningsen +Reviewed-By: Gabriel Schulhof +Reviewed-By: Michael Dawson +Reviewed-By: Colin Ihrig +Reviewed-By: Rich Trott +Reference: https://github.com/nodejs/node/commit/656260b4b65fec3b10f6da3fdc9f11fb941aafb5 +--- + src/node_api.cc | 12 +++++++++--- + test/addons-napi/test_string/test.js | 2 ++ + test/addons-napi/test_string/test_string.c | 20 ++++++++++++++++++++ + 3 files changed, 31 insertions(+), 3 deletions(-) + +diff --git a/src/node_api.cc b/src/node_api.cc +index 3dd5b38..2590a4b 100644 +--- a/src/node_api.cc ++++ b/src/node_api.cc +@@ -2753,7 +2753,7 @@ napi_status napi_get_value_string_latin1(napi_env env, + if (!buf) { + CHECK_ARG(env, result); + *result = val.As()->Length(); +- } else { ++ } else if (bufsize != 0) { + int copied = + val.As()->WriteOneByte(env->isolate, + reinterpret_cast(buf), +@@ -2765,6 +2765,8 @@ napi_status napi_get_value_string_latin1(napi_env env, + if (result != nullptr) { + *result = copied; + } ++ } else if (result != nullptr) { ++ *result = 0; + } + + return napi_clear_last_error(env); +@@ -2792,7 +2794,7 @@ napi_status napi_get_value_string_utf8(napi_env env, + if (!buf) { + CHECK_ARG(env, result); + *result = val.As()->Utf8Length(env->isolate); +- } else { ++ } else if (bufsize != 0) { + int copied = val.As()->WriteUtf8( + env->isolate, + buf, +@@ -2804,6 +2806,8 @@ napi_status napi_get_value_string_utf8(napi_env env, + if (result != nullptr) { + *result = copied; + } ++ } else if (result != nullptr) { ++ *result = 0; + } + + return napi_clear_last_error(env); +@@ -2832,7 +2836,7 @@ napi_status napi_get_value_string_utf16(napi_env env, + CHECK_ARG(env, result); + // V8 assumes UTF-16 length is the same as the number of characters. + *result = val.As()->Length(); +- } else { ++ } else if (bufsize != 0) { + int copied = val.As()->Write(env->isolate, + reinterpret_cast(buf), + 0, +@@ -2843,6 +2847,8 @@ napi_status napi_get_value_string_utf16(napi_env env, + if (result != nullptr) { + *result = copied; + } ++ } else if (result != nullptr) { ++ *result = 0; + } + + return napi_clear_last_error(env); +diff --git a/test/addons-napi/test_string/test.js b/test/addons-napi/test_string/test.js +index 5ce3d73..11c450f 100644 +--- a/test/addons-napi/test_string/test.js ++++ b/test/addons-napi/test_string/test.js +@@ -73,3 +73,5 @@ assert.strictEqual(test_string.Utf8Length(str6), 14); + assert.throws(() => { + test_string.TestLargeUtf8(); + }, /^Error: Invalid argument$/); ++ ++test_string.TestMemoryCorruption(' '.repeat(64 * 1024)); +diff --git a/test/addons-napi/test_string/test_string.c b/test/addons-napi/test_string/test_string.c +index 4e6da7b..ae16179 100644 +--- a/test/addons-napi/test_string/test_string.c ++++ b/test/addons-napi/test_string/test_string.c +@@ -1,4 +1,5 @@ + #include // INT_MAX ++#include + #include + #include "../common.h" + +@@ -216,6 +217,24 @@ static napi_value TestLargeUtf8(napi_env env, napi_callback_info info) { + return output; + } + ++static napi_value TestMemoryCorruption(napi_env env, napi_callback_info info) { ++ size_t argc = 1; ++ napi_value args[1]; ++ NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); ++ ++ NAPI_ASSERT(env, argc == 1, "Wrong number of arguments"); ++ ++ char buf[10] = { 0 }; ++ NAPI_CALL(env, napi_get_value_string_utf8(env, args[0], buf, 0, NULL)); ++ ++ char zero[10] = { 0 }; ++ if (memcmp(buf, zero, sizeof(buf)) != 0) { ++ NAPI_CALL(env, napi_throw_error(env, NULL, "Buffer overwritten")); ++ } ++ ++ return NULL; ++} ++ + static napi_value Init(napi_env env, napi_value exports) { + napi_property_descriptor properties[] = { + DECLARE_NAPI_PROPERTY("TestLatin1", TestLatin1), +@@ -227,6 +246,7 @@ static napi_value Init(napi_env env, napi_value exports) { + DECLARE_NAPI_PROPERTY("Utf16Length", Utf16Length), + DECLARE_NAPI_PROPERTY("Utf8Length", Utf8Length), + DECLARE_NAPI_PROPERTY("TestLargeUtf8", TestLargeUtf8), ++ DECLARE_NAPI_PROPERTY("TestMemoryCorruption", TestMemoryCorruption), + }; + + NAPI_CALL(env, napi_define_properties( +-- +2.23.0 + diff --git a/CVE-2020-8265.patch b/CVE-2020-8265.patch new file mode 100644 index 0000000000000000000000000000000000000000..b6eded25c046076b1adc5dc5f1c074dcd9a89755 --- /dev/null +++ b/CVE-2020-8265.patch @@ -0,0 +1,281 @@ +From 7f178663ebffc82c9f8a5a1b6bf2da0c263a30ed Mon Sep 17 00:00:00 2001 +From: Daniel Bevenius +Date: Wed, 2 Dec 2020 18:21:41 +0100 +Subject: [PATCH] src: use unique_ptr for WriteWrap +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +This commit attempts to avoid a use-after-free error by using unqiue_ptr +and passing a reference to it. + +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/238 +Reviewed-By: Michael Dawson +Reviewed-By: Tobias Nießen +Reviewed-By: Richard Lau +Reference: https://github.com/nodejs/node/commit/7f178663ebffc82c9f8a5a1b6bf2da0c263a30ed +--- + src/js_stream.cc | 4 ++-- + src/js_stream.h | 2 +- + src/node_file.h | 2 +- + src/node_http2.cc | 4 ++-- + src/node_http2.h | 2 +- + src/stream_base-inl.h | 8 ++++---- + src/stream_base.h | 9 +++++---- + src/stream_wrap.cc | 4 ++-- + src/stream_wrap.h | 2 +- + src/tls_wrap.cc | 13 +++++++------ + src/tls_wrap.h | 4 ++-- + 11 files changed, 28 insertions(+), 26 deletions(-) + +diff --git a/src/js_stream.cc b/src/js_stream.cc +index 4769a9c..f70b26b 100644 +--- a/src/js_stream.cc ++++ b/src/js_stream.cc +@@ -105,7 +105,7 @@ int JSStream::DoShutdown(ShutdownWrap* req_wrap) { + } + + +-int JSStream::DoWrite(WriteWrap* w, ++int JSStream::DoWrite(std::unique_ptr& w, + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle) { +@@ -122,7 +122,7 @@ int JSStream::DoWrite(WriteWrap* w, + } + + Local argv[] = { +- w->object(), ++ w.get()->object(), + bufs_arr + }; + +diff --git a/src/js_stream.h b/src/js_stream.h +index 05fb688..9e4b5a8 100644 +--- a/src/js_stream.h ++++ b/src/js_stream.h +@@ -22,7 +22,7 @@ class JSStream : public AsyncWrap, public StreamBase { + int ReadStop() override; + + int DoShutdown(ShutdownWrap* req_wrap) override; +- int DoWrite(WriteWrap* w, ++ int DoWrite(std::unique_ptr& w, + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle) override; +diff --git a/src/node_file.h b/src/node_file.h +index 73202d9..fc5475e 100644 +--- a/src/node_file.h ++++ b/src/node_file.h +@@ -246,7 +246,7 @@ class FileHandle : public AsyncWrap, public StreamBase { + ShutdownWrap* CreateShutdownWrap(v8::Local object) override; + int DoShutdown(ShutdownWrap* req_wrap) override; + +- int DoWrite(WriteWrap* w, ++ int DoWrite(std::unique_ptr& w, + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle) override { +diff --git a/src/node_http2.cc b/src/node_http2.cc +index 8841601..5ce2af0 100644 +--- a/src/node_http2.cc ++++ b/src/node_http2.cc +@@ -2063,7 +2063,7 @@ int Http2Stream::ReadStop() { + // chunks of data have been flushed to the underlying nghttp2_session. + // Note that this does *not* mean that the data has been flushed + // to the socket yet. +-int Http2Stream::DoWrite(WriteWrap* req_wrap, ++int Http2Stream::DoWrite(std::unique_ptr& req_wrap, + uv_buf_t* bufs, + size_t nbufs, + uv_stream_t* send_handle) { +@@ -2078,7 +2078,7 @@ int Http2Stream::DoWrite(WriteWrap* req_wrap, + // Store the req_wrap on the last write info in the queue, so that it is + // only marked as finished once all buffers associated with it are finished. + queue_.emplace(nghttp2_stream_write { +- i == nbufs - 1 ? req_wrap : nullptr, ++ i == nbufs - 1 ? req_wrap.get() : nullptr, + bufs[i] + }); + IncrementAvailableOutboundLength(bufs[i].len); +diff --git a/src/node_http2.h b/src/node_http2.h +index d7f8d9a..a327697 100644 +--- a/src/node_http2.h ++++ b/src/node_http2.h +@@ -566,7 +566,7 @@ class Http2Stream : public AsyncWrap, + + AsyncWrap* GetAsyncWrap() override { return this; } + +- int DoWrite(WriteWrap* w, uv_buf_t* bufs, size_t count, ++ int DoWrite(std::unique_ptr& w, uv_buf_t* bufs, size_t count, + uv_stream_t* send_handle) override; + + void MemoryInfo(MemoryTracker* tracker) const override { +diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h +index 027b938..dca02ac 100644 +--- a/src/stream_base-inl.h ++++ b/src/stream_base-inl.h +@@ -216,14 +216,14 @@ inline StreamWriteResult StreamBase::Write( + } + + AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(GetAsyncWrap()); +- WriteWrap* req_wrap = CreateWriteWrap(req_wrap_obj); ++ std::unique_ptr req_wrap{CreateWriteWrap(req_wrap_obj)}; + + err = DoWrite(req_wrap, bufs, count, send_handle); + bool async = err == 0; + +- if (!async) { ++ if (!async && req_wrap != nullptr) { + req_wrap->Dispose(); +- req_wrap = nullptr; ++ req_wrap.release(); + } + + const char* msg = Error(); +@@ -232,7 +232,7 @@ inline StreamWriteResult StreamBase::Write( + ClearError(); + } + +- return StreamWriteResult { async, err, req_wrap, total_bytes }; ++ return StreamWriteResult { async, err, req_wrap.release(), total_bytes }; + } + + template +diff --git a/src/stream_base.h b/src/stream_base.h +index 05c2a96..c8f8a0c 100644 +--- a/src/stream_base.h ++++ b/src/stream_base.h +@@ -211,10 +211,11 @@ class StreamResource { + // `*bufs` and `*count` accordingly. This is a no-op by default. + virtual int DoTryWrite(uv_buf_t** bufs, size_t* count); + // Perform a write of data, and call req_wrap->Done() when finished. +- virtual int DoWrite(WriteWrap* w, +- uv_buf_t* bufs, +- size_t count, +- uv_stream_t* send_handle) = 0; ++ virtual int DoWrite( ++ /* NOLINT (runtime/references) */ std::unique_ptr& w, ++ uv_buf_t* bufs, ++ size_t count, ++ uv_stream_t* send_handle) = 0; + + // Returns true if the stream supports the `OnStreamWantsWrite()` interface. + virtual bool HasWantsWrite() const { return false; } +diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc +index 60a1754..17cf861 100644 +--- a/src/stream_wrap.cc ++++ b/src/stream_wrap.cc +@@ -330,11 +330,11 @@ int LibuvStreamWrap::DoTryWrite(uv_buf_t** bufs, size_t* count) { + } + + +-int LibuvStreamWrap::DoWrite(WriteWrap* req_wrap, ++int LibuvStreamWrap::DoWrite(std::unique_ptr& req_wrap, + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle) { +- LibuvWriteWrap* w = static_cast(req_wrap); ++ LibuvWriteWrap* w = static_cast(req_wrap.get()); + int r; + if (send_handle == nullptr) { + r = w->Dispatch(uv_write, stream(), bufs, count, AfterUvWrite); +diff --git a/src/stream_wrap.h b/src/stream_wrap.h +index 487a40b..0d08683 100644 +--- a/src/stream_wrap.h ++++ b/src/stream_wrap.h +@@ -51,7 +51,7 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase { + // Resource implementation + int DoShutdown(ShutdownWrap* req_wrap) override; + int DoTryWrite(uv_buf_t** bufs, size_t* count) override; +- int DoWrite(WriteWrap* w, ++ int DoWrite(std::unique_ptr& w, + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle) override; +diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc +index eb40d85..eb85332 100644 +--- a/src/tls_wrap.cc ++++ b/src/tls_wrap.cc +@@ -93,8 +93,7 @@ bool TLSWrap::InvokeQueued(int status, const char* error_str) { + return false; + + if (current_write_ != nullptr) { +- WriteWrap* w = current_write_; +- current_write_ = nullptr; ++ WriteWrap* w = current_write_.release(); + w->Done(status, error_str); + } + +@@ -551,7 +550,7 @@ void TLSWrap::ClearError() { + } + + +-int TLSWrap::DoWrite(WriteWrap* w, ++int TLSWrap::DoWrite(std::unique_ptr& w, + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle) { +@@ -578,7 +577,7 @@ int TLSWrap::DoWrite(WriteWrap* w, + // the callback should not be invoked immediately + if (BIO_pending(enc_out_) == 0) { + CHECK_NULL(current_empty_write_); +- current_empty_write_ = w; ++ current_empty_write_ = w.get(); + StreamWriteResult res = + underlying_stream()->Write(bufs, count, send_handle); + if (!res.async) { +@@ -593,7 +592,7 @@ int TLSWrap::DoWrite(WriteWrap* w, + + // Store the current write wrap + CHECK_NULL(current_write_); +- current_write_ = w; ++ current_write_ = std::move(w); + + // Write queued data + if (empty) { +@@ -615,7 +614,7 @@ int TLSWrap::DoWrite(WriteWrap* w, + int err; + Local arg = GetSSLError(written, &err, &error_); + if (!arg.IsEmpty()) { +- current_write_ = nullptr; ++ current_write_.release(); + return UV_EPROTO; + } + +@@ -627,6 +626,8 @@ int TLSWrap::DoWrite(WriteWrap* w, + // Try writing data immediately + EncOut(); + ++ w.reset(current_write_.get()); ++ + return 0; + } + +diff --git a/src/tls_wrap.h b/src/tls_wrap.h +index aea8568..eb6b153 100644 +--- a/src/tls_wrap.h ++++ b/src/tls_wrap.h +@@ -67,7 +67,7 @@ class TLSWrap : public AsyncWrap, + ShutdownWrap* CreateShutdownWrap( + v8::Local req_wrap_object) override; + int DoShutdown(ShutdownWrap* req_wrap) override; +- int DoWrite(WriteWrap* w, ++ int DoWrite(std::unique_ptr& w, + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle) override; +@@ -149,7 +149,7 @@ class TLSWrap : public AsyncWrap, + BIO* enc_out_ = nullptr; + std::vector pending_cleartext_input_; + size_t write_size_; +- WriteWrap* current_write_ = nullptr; ++ std::unique_ptr current_write_ = nullptr; + WriteWrap* current_empty_write_ = nullptr; + bool write_callback_scheduled_ = false; + bool started_; +-- +2.23.0 + diff --git a/CVE-2020-8287.patch b/CVE-2020-8287.patch new file mode 100644 index 0000000000000000000000000000000000000000..41e2a98f945086e0ad1712f579a81567599c039d --- /dev/null +++ b/CVE-2020-8287.patch @@ -0,0 +1,79 @@ +From fc70ce08f5818a286fb5899a1bc3aff5965a745e 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/235 +Reviewed-By: Fedor Indutny +Reference: https://github.com/nodejs/node/commit/fc70ce08f5818a286fb5899a1bc3aff5965a745e +--- + deps/http_parser/http_parser.c | 6 ++++++ + deps/http_parser/test.c | 27 +++++++++++++++++++++++++++ + 2 files changed, 33 insertions(+) + +diff --git a/deps/http_parser/http_parser.c b/deps/http_parser/http_parser.c +index 6522618..bc785ce 100644 +--- a/deps/http_parser/http_parser.c ++++ b/deps/http_parser/http_parser.c +@@ -1311,6 +1311,12 @@ reexecute: + parser->header_state = h_general; + } else if (parser->index == sizeof(TRANSFER_ENCODING)-2) { + parser->header_state = h_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 cb445ce..99a6593 100644 +--- a/deps/http_parser/test.c ++++ b/deps/http_parser/test.c +@@ -1953,6 +1953,33 @@ const struct message responses[] = + ,.chunk_lengths= { 2, 2 } + } + ++#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 ++ } ++ + , {.name= NULL } /* sentinel */ + }; + +-- +2.23.0 + diff --git a/nodejs.spec b/nodejs.spec index f7e262bcffe1ec196e538707aa0077324a4466f6..bec134b4beb66902c63ff98916a5ae96e6bc0270 100644 --- a/nodejs.spec +++ b/nodejs.spec @@ -6,7 +6,7 @@ %global nodejs_patch 0 %global nodejs_abi %{nodejs_major}.%{nodejs_minor} %global nodejs_version %{nodejs_major}.%{nodejs_minor}.%{nodejs_patch} -%global nodejs_release 5 +%global nodejs_release 6 %global v8_major 6 %global v8_minor 8 @@ -46,6 +46,9 @@ Patch0004: CVE-2019-5737.patch Patch0005: CVE-2018-12121.patch Patch0006: CVE-2018-12123.patch Patch0007: src-avoid-OOB-read-in-URL-parser.patch +Patch0008: CVE-2020-8174.patch +Patch0009: CVE-2020-8265.patch +Patch0010: CVE-2020-8287.patch BuildRequires: gcc gcc-c++ openssl-devel BuildRequires: http-parser-devel @@ -222,6 +225,9 @@ NODE_PATH=%{buildroot}%{_prefix}/lib/node_modules:%{buildroot}%{_prefix}/lib/nod %doc %{_mandir}/man1/node.1* %changelog +* Thu Feb 04 2021 xinghe - 1:10.11.0-6 +- fix CVE-2020-8174 CVE-2020-8265 CVE-2020-8287 + * Wed Nov 04 2020 gaozhekang - 1:10.11.0-5 - avoid OOB read in URL parser