!180 [sync] PR-179: Fix CVE-2025-23085
From: @openeuler-sync-bot Reviewed-by: @wang--ge Signed-off-by: @wang--ge
This commit is contained in:
commit
7995eae96c
364
CVE-2025-23085.patch
Normal file
364
CVE-2025-23085.patch
Normal file
@ -0,0 +1,364 @@
|
|||||||
|
From: RafaelGSS <rafael.nunu@hotmail.com>
|
||||||
|
Date: Tue, 17 Dec 2024 16:58:03 -0300
|
||||||
|
Subject: [PATCH] src: fix HTTP2 mem leak on premature close and ERR_PROTO
|
||||||
|
|
||||||
|
This commit fixes a memory leak when the socket is
|
||||||
|
suddenly closed by the peer (without GOAWAY notification)
|
||||||
|
and when invalid header (by nghttp2) is identified and the
|
||||||
|
connection is terminated by peer.
|
||||||
|
|
||||||
|
[backport]
|
||||||
|
- test remove assert.strictEqual(session instanceof ServerHttp2Session, true); that fail on old version without ponyfill
|
||||||
|
- force non specific error in order to avoid an assert
|
||||||
|
|
||||||
|
Refs: https://hackerone.com/reports/2841362
|
||||||
|
PR-URL: https://github.com/nodejs-private/node-private/pull/650
|
||||||
|
Reviewed-By: James M Snell <jasnell@gmail.com>
|
||||||
|
CVE-ID: CVE-2025-23085
|
||||||
|
origin: backport, https://github.com/nodejs/node/commit/6cc8d58e6f97c37c228f134bd9b98246c8871fb1
|
||||||
|
bug: https://nodejs.org/en/blog/vulnerability/january-2025-security-releases#goaway-http2-frames-cause-memory-leak-outside-heap-cve-2025-23085---medium
|
||||||
|
---
|
||||||
|
lib/internal/http2/core.js | 14 +++-
|
||||||
|
src/node_http2.cc | 37 +++++++--
|
||||||
|
...-http2-connect-method-extended-cant-turn-off.js | 6 ++
|
||||||
|
test/parallel/test-http2-invalid-last-stream-id.js | 76 +++++++++++++++++++
|
||||||
|
.../test-http2-options-max-headers-block-length.js | 2 +
|
||||||
|
test/parallel/test-http2-premature-close.js | 88 ++++++++++++++++++++++
|
||||||
|
6 files changed, 216 insertions(+), 7 deletions(-)
|
||||||
|
create mode 100644 test/parallel/test-http2-invalid-last-stream-id.js
|
||||||
|
create mode 100644 test/parallel/test-http2-premature-close.js
|
||||||
|
|
||||||
|
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
|
||||||
|
index 1c6767c..d7caae3 100644
|
||||||
|
--- a/lib/internal/http2/core.js
|
||||||
|
+++ b/lib/internal/http2/core.js
|
||||||
|
@@ -573,9 +573,21 @@ function onFrameError(id, type, code) {
|
||||||
|
return;
|
||||||
|
debugSessionObj(session, 'error sending frame type %d on stream %d, code: %d',
|
||||||
|
type, id, code);
|
||||||
|
- const emitter = session[kState].streams.get(id) || session;
|
||||||
|
+
|
||||||
|
+ const stream = session[kState].streams.get(id);
|
||||||
|
+ const emitter = stream || session;
|
||||||
|
emitter[kUpdateTimer]();
|
||||||
|
emitter.emit('frameError', type, code, id);
|
||||||
|
+
|
||||||
|
+ // When a frameError happens is not uncommon that a pending GOAWAY
|
||||||
|
+ // package from nghttp2 is on flight with a correct error code.
|
||||||
|
+ // We schedule it using setImmediate to give some time for that
|
||||||
|
+ // package to arrive.
|
||||||
|
+ setImmediate(() => {
|
||||||
|
+ if(stream)
|
||||||
|
+ stream.close(NGHTTP2_INTERNAL_ERROR);
|
||||||
|
+ session.close();
|
||||||
|
+ });
|
||||||
|
}
|
||||||
|
|
||||||
|
function onAltSvc(stream, origin, alt) {
|
||||||
|
diff --git a/src/node_http2.cc b/src/node_http2.cc
|
||||||
|
index c441921..6365734 100644
|
||||||
|
--- a/src/node_http2.cc
|
||||||
|
+++ b/src/node_http2.cc
|
||||||
|
@@ -742,6 +742,7 @@ inline bool Http2Session::CanAddStream() {
|
||||||
|
}
|
||||||
|
|
||||||
|
inline void Http2Session::AddStream(Http2Stream* stream) {
|
||||||
|
+ Debug(this, "Adding stream: %d", stream->id());
|
||||||
|
CHECK_GE(++statistics_.stream_count, 0);
|
||||||
|
streams_[stream->id()] = stream;
|
||||||
|
size_t size = streams_.size();
|
||||||
|
@@ -750,10 +751,10 @@ inline void Http2Session::AddStream(Http2Stream* stream) {
|
||||||
|
IncrementCurrentSessionMemory(sizeof(*stream));
|
||||||
|
}
|
||||||
|
|
||||||
|
-
|
||||||
|
inline void Http2Session::RemoveStream(Http2Stream* stream) {
|
||||||
|
if (streams_.empty() || stream == nullptr)
|
||||||
|
return; // Nothing to remove, item was never added?
|
||||||
|
+ Debug(this, "Removing stream: %d", stream->id());
|
||||||
|
streams_.erase(stream->id());
|
||||||
|
DecrementCurrentSessionMemory(sizeof(*stream));
|
||||||
|
}
|
||||||
|
@@ -926,6 +927,7 @@ int Http2Session::OnHeaderCallback(nghttp2_session* handle,
|
||||||
|
if (UNLIKELY(stream == nullptr))
|
||||||
|
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
|
||||||
|
|
||||||
|
+ Debug(session, "handling header key/pair for stream %d", id);
|
||||||
|
// If the stream has already been destroyed, ignore.
|
||||||
|
if (!stream->IsDestroyed() && !stream->AddHeader(name, value, flags)) {
|
||||||
|
// This will only happen if the connected peer sends us more
|
||||||
|
@@ -995,9 +997,21 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
- // If the error is fatal or if error code is ERR_STREAM_CLOSED... emit error
|
||||||
|
+ // If the error is fatal or if error code is one of the following
|
||||||
|
+ // we emit and error:
|
||||||
|
+ //
|
||||||
|
+ // ERR_STREAM_CLOSED: An invalid frame has been received in a closed stream.
|
||||||
|
+ //
|
||||||
|
+ // ERR_PROTO: The RFC 7540 specifies:
|
||||||
|
+ // "An endpoint that encounters a connection error SHOULD first send a GOAWAY
|
||||||
|
+ // frame (Section 6.8) with the stream identifier of the last stream that it
|
||||||
|
+ // successfully received from its peer.
|
||||||
|
+ // The GOAWAY frame includes an error code that indicates the type of error"
|
||||||
|
+ // The GOAWAY frame is already sent by nghttp2. We emit the error
|
||||||
|
+ // to liberate the Http2Session to destroy.
|
||||||
|
if (nghttp2_is_fatal(lib_error_code) ||
|
||||||
|
- lib_error_code == NGHTTP2_ERR_STREAM_CLOSED) {
|
||||||
|
+ lib_error_code == NGHTTP2_ERR_STREAM_CLOSED ||
|
||||||
|
+ lib_error_code == NGHTTP2_ERR_PROTO) {
|
||||||
|
Environment* env = session->env();
|
||||||
|
Isolate* isolate = env->isolate();
|
||||||
|
HandleScope scope(isolate);
|
||||||
|
@@ -1024,12 +1038,19 @@ int Http2Session::OnFrameNotSent(nghttp2_session* handle,
|
||||||
|
Debug(session, "frame type %d was not sent, code: %d",
|
||||||
|
frame->hd.type, error_code);
|
||||||
|
|
||||||
|
- // Do not report if the frame was not sent due to the session closing
|
||||||
|
if (error_code == NGHTTP2_ERR_SESSION_CLOSING ||
|
||||||
|
error_code == NGHTTP2_ERR_STREAM_CLOSED ||
|
||||||
|
error_code == NGHTTP2_ERR_STREAM_CLOSING ||
|
||||||
|
session->js_fields_.frame_error_listener_count == 0) {
|
||||||
|
- return 0;
|
||||||
|
+ // Currently, nghttp2 doesn't not inform us when is the best
|
||||||
|
+ // time to call session.close(). It relies on a closing connection
|
||||||
|
+ // from peer. If that doesn't happen, the nghttp2_session will be
|
||||||
|
+ // closed but the Http2Session will still be up causing a memory leak.
|
||||||
|
+ // Therefore, if the GOAWAY frame couldn't be send due to
|
||||||
|
+ // ERR_SESSION_CLOSING we should force close from our side.
|
||||||
|
+ if (frame->hd.type != 0x03) {
|
||||||
|
+ return 0;
|
||||||
|
+ }
|
||||||
|
}
|
||||||
|
|
||||||
|
Isolate* isolate = env->isolate();
|
||||||
|
@@ -1095,12 +1116,15 @@ int Http2Session::OnStreamClose(nghttp2_session* handle,
|
||||||
|
// ignore these. If this callback was not provided, nghttp2 would handle
|
||||||
|
// invalid headers strictly and would shut down the stream. We are intentionally
|
||||||
|
// being more lenient here although we may want to revisit this choice later.
|
||||||
|
-int Http2Session::OnInvalidHeader(nghttp2_session* session,
|
||||||
|
+int Http2Session::OnInvalidHeader(nghttp2_session* handle,
|
||||||
|
const nghttp2_frame* frame,
|
||||||
|
nghttp2_rcbuf* name,
|
||||||
|
nghttp2_rcbuf* value,
|
||||||
|
uint8_t flags,
|
||||||
|
void* user_data) {
|
||||||
|
+ Http2Session* session = static_cast<Http2Session*>(user_data);
|
||||||
|
+ int32_t id = GetFrameID(frame);
|
||||||
|
+ Debug(session, "invalid header received for stream %d", id);
|
||||||
|
// Ignore invalid header fields by default.
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
@@ -1494,6 +1518,7 @@ void Http2Session::HandlePingFrame(const nghttp2_frame* frame) {
|
||||||
|
|
||||||
|
// Called by OnFrameReceived when a complete SETTINGS frame has been received.
|
||||||
|
void Http2Session::HandleSettingsFrame(const nghttp2_frame* frame) {
|
||||||
|
+ Debug(this, "handling settings frame");
|
||||||
|
bool ack = frame->hd.flags & NGHTTP2_FLAG_ACK;
|
||||||
|
if (!ack) {
|
||||||
|
js_fields_.bitfield &= ~(1 << kSessionRemoteSettingsIsUpToDate);
|
||||||
|
diff --git a/test/parallel/test-http2-connect-method-extended-cant-turn-off.js b/test/parallel/test-http2-connect-method-extended-cant-turn-off.js
|
||||||
|
index f4d033e..456aa1c 100644
|
||||||
|
--- a/test/parallel/test-http2-connect-method-extended-cant-turn-off.js
|
||||||
|
+++ b/test/parallel/test-http2-connect-method-extended-cant-turn-off.js
|
||||||
|
@@ -27,4 +27,10 @@ server.listen(0, common.mustCall(() => {
|
||||||
|
server.close();
|
||||||
|
}));
|
||||||
|
}));
|
||||||
|
+
|
||||||
|
+ client.on('error', common.expectsError({
|
||||||
|
+ code: 'ERR_HTTP2_ERROR',
|
||||||
|
+ name: 'Error',
|
||||||
|
+ message: 'Protocol error'
|
||||||
|
+ }));
|
||||||
|
}));
|
||||||
|
diff --git a/test/parallel/test-http2-invalid-last-stream-id.js b/test/parallel/test-http2-invalid-last-stream-id.js
|
||||||
|
new file mode 100644
|
||||||
|
index 0000000..8e7beae
|
||||||
|
--- /dev/null
|
||||||
|
+++ b/test/parallel/test-http2-invalid-last-stream-id.js
|
||||||
|
@@ -0,0 +1,76 @@
|
||||||
|
+// Flags: --expose-internals
|
||||||
|
+'use strict';
|
||||||
|
+
|
||||||
|
+const common = require('../common');
|
||||||
|
+if (!common.hasCrypto) common.skip('missing crypto');
|
||||||
|
+
|
||||||
|
+const h2 = require('http2');
|
||||||
|
+const net = require('net');
|
||||||
|
+const assert = require('assert');
|
||||||
|
+const { ServerHttp2Session } = require('internal/http2/core');
|
||||||
|
+
|
||||||
|
+async function sendInvalidLastStreamId(server) {
|
||||||
|
+ const client = new net.Socket();
|
||||||
|
+
|
||||||
|
+ const address = server.address();
|
||||||
|
+ if (!common.hasIPv6 && address.family === 'IPv6') {
|
||||||
|
+ // Necessary to pass CI running inside containers.
|
||||||
|
+ client.connect(address.port);
|
||||||
|
+ } else {
|
||||||
|
+ client.connect(address);
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ client.on('connect', common.mustCall(function() {
|
||||||
|
+ // HTTP/2 preface
|
||||||
|
+ client.write(Buffer.from('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n', 'utf8'));
|
||||||
|
+
|
||||||
|
+ // Empty SETTINGS frame
|
||||||
|
+ client.write(Buffer.from([0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00]));
|
||||||
|
+
|
||||||
|
+ // GOAWAY frame with custom debug message
|
||||||
|
+ const goAwayFrame = [
|
||||||
|
+ 0x00, 0x00, 0x21, // Length: 33 bytes
|
||||||
|
+ 0x07, // Type: GOAWAY
|
||||||
|
+ 0x00, // Flags
|
||||||
|
+ 0x00, 0x00, 0x00, 0x00, // Stream ID: 0
|
||||||
|
+ 0x00, 0x00, 0x00, 0x01, // Last Stream ID: 1
|
||||||
|
+ 0x00, 0x00, 0x00, 0x00, // Error Code: 0 (No error)
|
||||||
|
+ ];
|
||||||
|
+
|
||||||
|
+ // Add the debug message
|
||||||
|
+ const debugMessage = 'client transport shutdown';
|
||||||
|
+ const goAwayBuffer = Buffer.concat([
|
||||||
|
+ Buffer.from(goAwayFrame),
|
||||||
|
+ Buffer.from(debugMessage, 'utf8'),
|
||||||
|
+ ]);
|
||||||
|
+
|
||||||
|
+ client.write(goAwayBuffer);
|
||||||
|
+ client.destroy();
|
||||||
|
+ }));
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+const server = h2.createServer();
|
||||||
|
+
|
||||||
|
+server.on('error', common.mustNotCall());
|
||||||
|
+
|
||||||
|
+server.on(
|
||||||
|
+ 'sessionError',
|
||||||
|
+ common.mustCall((err, session) => {
|
||||||
|
+ // When destroying the session, on Windows, we would get ECONNRESET
|
||||||
|
+ // errors, make sure we take those into account in our tests.
|
||||||
|
+ if (err.code !== 'ECONNRESET') {
|
||||||
|
+ assert.strictEqual(err.code, 'ERR_HTTP2_ERROR');
|
||||||
|
+ assert.strictEqual(err.name, 'Error');
|
||||||
|
+ assert.strictEqual(err.message, 'Protocol error');
|
||||||
|
+ }
|
||||||
|
+ session.close();
|
||||||
|
+ server.close();
|
||||||
|
+ }),
|
||||||
|
+);
|
||||||
|
+
|
||||||
|
+server.listen(
|
||||||
|
+ 0,
|
||||||
|
+ common.mustCall(async () => {
|
||||||
|
+ await sendInvalidLastStreamId(server);
|
||||||
|
+ }),
|
||||||
|
+);
|
||||||
|
diff --git a/test/parallel/test-http2-options-max-headers-block-length.js b/test/parallel/test-http2-options-max-headers-block-length.js
|
||||||
|
index 11632c6..a74e1e1 100644
|
||||||
|
--- a/test/parallel/test-http2-options-max-headers-block-length.js
|
||||||
|
+++ b/test/parallel/test-http2-options-max-headers-block-length.js
|
||||||
|
@@ -35,6 +35,8 @@ server.listen(0, common.mustCall(() => {
|
||||||
|
assert.strictEqual(code, h2.constants.NGHTTP2_ERR_FRAME_SIZE_ERROR);
|
||||||
|
}));
|
||||||
|
|
||||||
|
+ // NGHTTP2 will automatically send the NGHTTP2_REFUSED_STREAM with
|
||||||
|
+ // the GOAWAY frame.
|
||||||
|
req.on('error', common.expectsError({
|
||||||
|
code: 'ERR_HTTP2_STREAM_ERROR',
|
||||||
|
name: 'Error',
|
||||||
|
diff --git a/test/parallel/test-http2-premature-close.js b/test/parallel/test-http2-premature-close.js
|
||||||
|
new file mode 100644
|
||||||
|
index 0000000..a9b08f5
|
||||||
|
--- /dev/null
|
||||||
|
+++ b/test/parallel/test-http2-premature-close.js
|
||||||
|
@@ -0,0 +1,88 @@
|
||||||
|
+// Flags: --expose-internals
|
||||||
|
+'use strict';
|
||||||
|
+
|
||||||
|
+const common = require('../common');
|
||||||
|
+if (!common.hasCrypto) common.skip('missing crypto');
|
||||||
|
+
|
||||||
|
+const h2 = require('http2');
|
||||||
|
+const net = require('net');
|
||||||
|
+
|
||||||
|
+async function requestAndClose(server) {
|
||||||
|
+ const client = new net.Socket();
|
||||||
|
+
|
||||||
|
+ const address = server.address();
|
||||||
|
+ if (!common.hasIPv6 && address.family === 'IPv6') {
|
||||||
|
+ // Necessary to pass CI running inside containers.
|
||||||
|
+ client.connect(address.port);
|
||||||
|
+ } else {
|
||||||
|
+ client.connect(address);
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ client.on('connect', common.mustCall(function() {
|
||||||
|
+ // Send HTTP/2 Preface
|
||||||
|
+ client.write(Buffer.from('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n', 'utf8'));
|
||||||
|
+
|
||||||
|
+ // Send a SETTINGS frame (empty payload)
|
||||||
|
+ client.write(Buffer.from([0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00]));
|
||||||
|
+
|
||||||
|
+ const streamId = 1;
|
||||||
|
+ // Send a valid HEADERS frame
|
||||||
|
+ const headersFrame = Buffer.concat([
|
||||||
|
+ Buffer.from([
|
||||||
|
+ 0x00, 0x00, 0x0c, // Length: 12 bytes
|
||||||
|
+ 0x01, // Type: HEADERS
|
||||||
|
+ 0x05, // Flags: END_HEADERS + END_STREAM
|
||||||
|
+ (streamId >> 24) & 0xFF, // Stream ID: high byte
|
||||||
|
+ (streamId >> 16) & 0xFF,
|
||||||
|
+ (streamId >> 8) & 0xFF,
|
||||||
|
+ streamId & 0xFF, // Stream ID: low byte
|
||||||
|
+ ]),
|
||||||
|
+ Buffer.from([
|
||||||
|
+ 0x82, // Indexed Header Field Representation (Predefined ":method: GET")
|
||||||
|
+ 0x84, // Indexed Header Field Representation (Predefined ":path: /")
|
||||||
|
+ 0x86, // Indexed Header Field Representation (Predefined ":scheme: http")
|
||||||
|
+ 0x44, 0x0a, // Custom ":authority: localhost"
|
||||||
|
+ 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x68, 0x6f, 0x73, 0x74,
|
||||||
|
+ ]),
|
||||||
|
+ ]);
|
||||||
|
+ client.write(headersFrame);
|
||||||
|
+
|
||||||
|
+ // Send a valid DATA frame
|
||||||
|
+ const dataFrame = Buffer.concat([
|
||||||
|
+ Buffer.from([
|
||||||
|
+ 0x00, 0x00, 0x05, // Length: 5 bytes
|
||||||
|
+ 0x00, // Type: DATA
|
||||||
|
+ 0x00, // Flags: No flags
|
||||||
|
+ (streamId >> 24) & 0xFF, // Stream ID: high byte
|
||||||
|
+ (streamId >> 16) & 0xFF,
|
||||||
|
+ (streamId >> 8) & 0xFF,
|
||||||
|
+ streamId & 0xFF, // Stream ID: low byte
|
||||||
|
+ ]),
|
||||||
|
+ Buffer.from('Hello', 'utf8'), // Data payload
|
||||||
|
+ ]);
|
||||||
|
+ client.write(dataFrame);
|
||||||
|
+
|
||||||
|
+ // Does not wait for server to reply. Shutdown the socket
|
||||||
|
+ client.end();
|
||||||
|
+ }));
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+const server = h2.createServer();
|
||||||
|
+
|
||||||
|
+server.on('error', common.mustNotCall());
|
||||||
|
+
|
||||||
|
+server.on(
|
||||||
|
+ 'session',
|
||||||
|
+ common.mustCall((session) => {
|
||||||
|
+ session.on('close', common.mustCall(() => {
|
||||||
|
+ server.close();
|
||||||
|
+ }));
|
||||||
|
+ }),
|
||||||
|
+);
|
||||||
|
+
|
||||||
|
+server.listen(
|
||||||
|
+ 0,
|
||||||
|
+ common.mustCall(async () => {
|
||||||
|
+ await requestAndClose(server);
|
||||||
|
+ }),
|
||||||
|
+);
|
||||||
@ -1,5 +1,5 @@
|
|||||||
%bcond_with bootstrap
|
%bcond_with bootstrap
|
||||||
%global baserelease 10
|
%global baserelease 11
|
||||||
%{?!_pkgdocdir:%global _pkgdocdir %{_docdir}/%{name}-%{version}}
|
%{?!_pkgdocdir:%global _pkgdocdir %{_docdir}/%{name}-%{version}}
|
||||||
%global nodejs_epoch 1
|
%global nodejs_epoch 1
|
||||||
%global nodejs_major 12
|
%global nodejs_major 12
|
||||||
@ -111,6 +111,7 @@ Patch00032: CVE-2024-22019.patch
|
|||||||
Patch00033: CVE-2024-22025.patch
|
Patch00033: CVE-2024-22025.patch
|
||||||
Patch00034: CVE-2024-27982.patch
|
Patch00034: CVE-2024-27982.patch
|
||||||
Patch00035: CVE-2024-27983.patch
|
Patch00035: CVE-2024-27983.patch
|
||||||
|
Patch00036: CVE-2025-23085.patch
|
||||||
|
|
||||||
BuildRequires: python3-devel
|
BuildRequires: python3-devel
|
||||||
BuildRequires: zlib-devel
|
BuildRequires: zlib-devel
|
||||||
@ -513,6 +514,9 @@ end
|
|||||||
%{_pkgdocdir}/npm/docs
|
%{_pkgdocdir}/npm/docs
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Wed Mar 05 2025 yaoxin <1024769339@qq.com> - 1:12.22.11-11
|
||||||
|
- Fix CVE-2025-23085
|
||||||
|
|
||||||
* Thu Sep 19 2024 yaoxin <yao_xin001@hoperun.com>- 1:12.22.11-10
|
* Thu Sep 19 2024 yaoxin <yao_xin001@hoperun.com>- 1:12.22.11-10
|
||||||
- Fix CVE-2023-46809,CVE-2024-22019,CVE-2024-22025,CVE-2024-27982 and CVE-2024-27983
|
- Fix CVE-2023-46809,CVE-2024-22019,CVE-2024-22025,CVE-2024-27982 and CVE-2024-27983
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user