diff --git a/CVE-2020-8265.patch b/CVE-2020-8265.patch new file mode 100644 index 0000000000000000000000000000000000000000..8720755ce29dd43ce9eb6555637052da51cec48b --- /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 e3d734c..4054e90 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 6612e55..bf0d15d 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 cbbb8b0..b440c14 100644 +--- a/src/node_file.h ++++ b/src/node_file.h +@@ -287,7 +287,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 9bde444..2a523a4 100644 +--- a/src/node_http2.cc ++++ b/src/node_http2.cc +@@ -2314,7 +2314,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) { +@@ -2329,7 +2329,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 1526e0b..d1d523e 100644 +--- a/src/node_http2.h ++++ b/src/node_http2.h +@@ -568,7 +568,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 65abd4d..3e922a4 100644 +--- a/src/stream_base.h ++++ b/src/stream_base.h +@@ -215,10 +215,11 @@ class StreamResource { + virtual int DoTryWrite(uv_buf_t** bufs, size_t* count); + // Perform a write of data, and either call req_wrap->Done() when finished + // and return 0, or return a libuv error code for synchronous failures. +- 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 10444fe..bd512e3 100644 +--- a/src/stream_wrap.cc ++++ b/src/stream_wrap.cc +@@ -351,11 +351,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 98f0ca4..3c00d33 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 ce46e21..65ea884 100644 +--- a/src/tls_wrap.cc ++++ b/src/tls_wrap.cc +@@ -91,8 +91,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); + } + +@@ -617,7 +616,7 @@ void TLSWrap::ClearError() { + + + // Called by StreamBase::Write() to request async write of clear text into SSL. +-int TLSWrap::DoWrite(WriteWrap* w, ++int TLSWrap::DoWrite(std::unique_ptr& w, + uv_buf_t* bufs, + size_t count, + uv_stream_t* send_handle) { +@@ -651,7 +650,7 @@ int TLSWrap::DoWrite(WriteWrap* w, + if (BIO_pending(enc_out_) == 0) { + Debug(this, "No pending encrypted output, writing to underlying stream"); + 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) { +@@ -666,7 +665,7 @@ int TLSWrap::DoWrite(WriteWrap* w, + + // Store the current write wrap + CHECK_NULL(current_write_); +- current_write_ = w; ++ current_write_ = std::move(w); + + // Write encrypted data to underlying stream and call Done(). + if (length == 0) { +@@ -705,7 +704,7 @@ int TLSWrap::DoWrite(WriteWrap* w, + // If we stopped writing because of an error, it's fatal, discard the data. + if (!arg.IsEmpty()) { + Debug(this, "Got SSL error (%d), returning UV_EPROTO", err); +- current_write_ = nullptr; ++ current_write_.release(); + return UV_EPROTO; + } + +@@ -718,6 +717,8 @@ int TLSWrap::DoWrite(WriteWrap* w, + // Write any encrypted/handshake output that may be ready. + EncOut(); + ++ w.reset(current_write_.get()); ++ + return 0; + } + +diff --git a/src/tls_wrap.h b/src/tls_wrap.h +index bfcf07b..e2e748b 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; +@@ -170,7 +170,7 @@ class TLSWrap : public AsyncWrap, + // Waiting for ClearIn() to pass to SSL_write(). + std::vector pending_cleartext_input_; + size_t write_size_ = 0; +- WriteWrap* current_write_ = nullptr; ++ std::unique_ptr current_write_ = nullptr; + WriteWrap* current_empty_write_ = nullptr; + bool write_callback_scheduled_ = false; + bool started_ = false; +-- +2.23.0 + diff --git a/CVE-2020-8287.patch b/CVE-2020-8287.patch new file mode 100644 index 0000000000000000000000000000000000000000..2d3e08c4f70572a81c95f2997277459cdf4059f4 --- /dev/null +++ b/CVE-2020-8287.patch @@ -0,0 +1,78 @@ +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 | 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/nodejs.spec b/nodejs.spec index 3702257bf0544f51b6b6f3bc64b1328e650965c8..973a1afce4cca5441a8d3e750c41409b96954dc1 100644 --- a/nodejs.spec +++ b/nodejs.spec @@ -1,5 +1,5 @@ %bcond_with bootstrap -%global baserelease 4 +%global baserelease 5 %{?!_pkgdocdir:%global _pkgdocdir %{_docdir}/%{name}-%{version}} %global nodejs_epoch 1 %global nodejs_major 10 @@ -74,6 +74,8 @@ Patch2: 0002-Install-both-binaries-and-use-libdir.patch Patch3: 0003-build-auto-load-ICU-data-from-with-icu-default-data-.patch Patch4: 0004-src-avoid-OOB-read-in-URL-parser.patch Patch5: CVE-2020-8252.patch +Patch6: CVE-2020-8265.patch +Patch7: CVE-2020-8287.patch BuildRequires: python2-devel python3-devel zlib-devel gcc >= 6.3.0 BuildRequires: gcc-c++ >= 6.3.0 nodejs-packaging chrpath libatomic @@ -461,6 +463,9 @@ end %changelog +* Fri Feb 5 2021 xinghe 1:10.21.0-5 +- fix CVE-2020-8265 CVE-2020-8287 + * Tue Dec 6 2020 wangxiao 1:10.21.0-4 - fix CVE-2020-8252