From fc07177109b59a6f4129818222f1280e51891d30 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Fri, 31 Oct 2025 16:27:48 -0300 Subject: [PATCH 1/6] lib: add TLSSocket default error handler This prevents the server from crashing due to an unhandled rejection when a TLSSocket connection is abruptly destroyed during initialization and the user has not attached an error handler to the socket. e.g: ```js const server = http2.createSecureServer({ ... }) server.on('secureConnection', socket => { socket.on('error', err => { console.log(err) }) }) ``` PR-URL: https://github.com/nodejs-private/node-private/pull/797 Fixes: https://github.com/nodejs/node/issues/44751 Refs: https://hackerone.com/bugs?subject=nodejs&report_id=3262404 Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen CVE-ID: CVE-2025-59465 --- lib/_tls_wrap.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index fe281d0c5d9dff..c5e2b6ff26d951 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -1090,6 +1090,7 @@ function tlsConnectionListener(rawSocket) { socket[kErrorEmitted] = false; socket.on('close', onSocketClose); socket.on('_tlsError', onSocketTLSError); + socket.on('error', onSocketTLSError); } // AUTHENTICATION MODES From 799755a810952c940f4984cd980bdeb298c1ec86 Mon Sep 17 00:00:00 2001 From: Erik Olofsson Date: Mon, 19 Jan 2026 14:48:42 +0100 Subject: [PATCH 2/6] Fix compile issues --- deps/v8/third_party/zlib/zutil.h | 2 +- deps/zlib/zutil.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/deps/v8/third_party/zlib/zutil.h b/deps/v8/third_party/zlib/zutil.h index 4425bcf75eb38c..45bf18e2d32ce7 100644 --- a/deps/v8/third_party/zlib/zutil.h +++ b/deps/v8/third_party/zlib/zutil.h @@ -145,7 +145,7 @@ extern z_const char * const z_errmsg[10]; /* indexed by 2-zlib_error */ # endif #endif -#if defined(MACOS) || defined(TARGET_OS_MAC) +#if (defined(MACOS) || defined(TARGET_OS_MAC)) && !defined(__APPLE__) # define OS_CODE 7 # ifndef Z_SOLO # if defined(__MWERKS__) && __dest_os != __be_os && __dest_os != __win32_os diff --git a/deps/zlib/zutil.h b/deps/zlib/zutil.h index 4425bcf75eb38c..45bf18e2d32ce7 100644 --- a/deps/zlib/zutil.h +++ b/deps/zlib/zutil.h @@ -145,7 +145,7 @@ extern z_const char * const z_errmsg[10]; /* indexed by 2-zlib_error */ # endif #endif -#if defined(MACOS) || defined(TARGET_OS_MAC) +#if (defined(MACOS) || defined(TARGET_OS_MAC)) && !defined(__APPLE__) # define OS_CODE 7 # ifndef Z_SOLO # if defined(__MWERKS__) && __dest_os != __be_os && __dest_os != __win32_os From 04f5238d5a1bc2c7eef4f39c710a60bf4852fb57 Mon Sep 17 00:00:00 2001 From: Erik Olofsson Date: Mon, 19 Jan 2026 15:27:13 +0100 Subject: [PATCH 3/6] Fix compile with BoringSSL --- common.gypi | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common.gypi b/common.gypi index b1b59776a12c75..c2bd95365312ba 100644 --- a/common.gypi +++ b/common.gypi @@ -564,6 +564,12 @@ 'OPENSSL_NO_ASM', ], }], + # Disable C++ features in BoringSSL headers + ['1==1', { + 'defines': [ + 'BORINGSSL_NO_CXX', + ], + }], ], } } From d225136ff3ff7d295b612cf617b477b7b3862a0c Mon Sep 17 00:00:00 2001 From: Erik Olofsson Date: Mon, 19 Jan 2026 13:10:20 +0100 Subject: [PATCH 4/6] CVE-2026-21637 --- lib/_tls_wrap.js | 102 +++++----- ...est-tls-psk-callback-exception-handling.js | 186 ++++++++++++++++++ 2 files changed, 243 insertions(+), 45 deletions(-) create mode 100644 test/parallel/test-tls-psk-callback-exception-handling.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index c5e2b6ff26d951..59739f83f63887 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -320,64 +320,76 @@ function onnewsession(sessionId, session) { function onPskServerCallback(identity, maxPskLen) { const owner = this[owner_symbol]; - const ret = owner[kPskCallback](owner, identity); - if (ret == null) - return undefined; - let psk; - if (isArrayBufferView(ret)) { - psk = ret; - } else { - if (typeof ret !== 'object') { - throw new ERR_INVALID_ARG_TYPE( - 'ret', - ['Object', 'Buffer', 'TypedArray', 'DataView'], - ret + try { + const ret = owner[kPskCallback](owner, identity); + if (ret == null) + return undefined; + + let psk; + if (isArrayBufferView(ret)) { + psk = ret; + } else { + if (typeof ret !== 'object') { + throw new ERR_INVALID_ARG_TYPE( + 'ret', + ['Object', 'Buffer', 'TypedArray', 'DataView'], + ret + ); + } + psk = ret.psk; + validateBuffer(psk, 'psk'); + } + + if (psk.length > maxPskLen) { + throw new ERR_INVALID_ARG_VALUE( + 'psk', + psk, + `Pre-shared key exceeds ${maxPskLen} bytes` ); } - psk = ret.psk; - validateBuffer(psk, 'psk'); - } - if (psk.length > maxPskLen) { - throw new ERR_INVALID_ARG_VALUE( - 'psk', - psk, - `Pre-shared key exceeds ${maxPskLen} bytes` - ); + return psk; + } catch (err) { + owner.destroy(err); + return undefined; } - - return psk; } function onPskClientCallback(hint, maxPskLen, maxIdentityLen) { const owner = this[owner_symbol]; - const ret = owner[kPskCallback](hint); - if (ret == null) - return undefined; - if (typeof ret !== 'object') - throw new ERR_INVALID_ARG_TYPE('ret', 'Object', ret); + try { + const ret = owner[kPskCallback](hint); + if (ret == null) + return undefined; + + if (typeof ret !== 'object') + throw new ERR_INVALID_ARG_TYPE('ret', 'Object', ret); + + validateBuffer(ret.psk, 'psk'); + if (ret.psk.length > maxPskLen) { + throw new ERR_INVALID_ARG_VALUE( + 'psk', + ret.psk, + `Pre-shared key exceeds ${maxPskLen} bytes` + ); + } - validateBuffer(ret.psk, 'psk'); - if (ret.psk.length > maxPskLen) { - throw new ERR_INVALID_ARG_VALUE( - 'psk', - ret.psk, - `Pre-shared key exceeds ${maxPskLen} bytes` - ); - } + validateString(ret.identity, 'identity'); + if (Buffer.byteLength(ret.identity) > maxIdentityLen) { + throw new ERR_INVALID_ARG_VALUE( + 'identity', + ret.identity, + `PSK identity exceeds ${maxIdentityLen} bytes` + ); + } - validateString(ret.identity, 'identity'); - if (Buffer.byteLength(ret.identity) > maxIdentityLen) { - throw new ERR_INVALID_ARG_VALUE( - 'identity', - ret.identity, - `PSK identity exceeds ${maxIdentityLen} bytes` - ); + return { psk: ret.psk, identity: ret.identity }; + } catch (err) { + owner.destroy(err); + return undefined; } - - return { psk: ret.psk, identity: ret.identity }; } function onkeylog(line) { diff --git a/test/parallel/test-tls-psk-callback-exception-handling.js b/test/parallel/test-tls-psk-callback-exception-handling.js new file mode 100644 index 00000000000000..73782e10da47e2 --- /dev/null +++ b/test/parallel/test-tls-psk-callback-exception-handling.js @@ -0,0 +1,186 @@ +'use strict'; + +// This test verifies that exceptions in pskCallback are properly routed +// through tlsClientError instead of becoming uncaught exceptions. +// This is a regression test for CVE-2026-21637. +// +// Note: ALPNCallback tests are not included because ALPNCallback option +// was added in Node.js v20+. + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const tls = require('tls'); + +const CIPHERS = 'PSK+HIGH'; + +// Run tests sequentially +const tests = []; +let testNumber = 0; + +function runNextTest() { + const test = tests.shift(); + if (test) { + testNumber++; + console.log(`Running test ${testNumber}...`); + test(runNextTest); + } else { + console.log('All tests passed!'); + } +} + +// Test 1: PSK server callback returning invalid type should emit tlsClientError +tests.push((done) => { + console.log(' - Server pskCallback returns invalid type -> tlsClientError'); + const server = tls.createServer({ + ciphers: CIPHERS, + pskCallback: common.mustCall(() => { + console.log(' pskCallback called, returning invalid string'); + // Return invalid type (string instead of object/Buffer) + return 'invalid-should-be-object-or-buffer'; + }), + pskIdentityHint: 'test-hint', + }); + + server.on('tlsClientError', common.mustCall((err, socket) => { + console.log(' tlsClientError received:', err.code); + assert.ok(err instanceof Error); + assert.strictEqual(err.code, 'ERR_INVALID_ARG_TYPE'); + socket.destroy(); + console.log(' Test 1 PASSED'); + server.close(done); + })); + + server.on('secureConnection', common.mustNotCall()); + + server.listen(0, common.mustCall(() => { + const client = tls.connect({ + port: server.address().port, + host: '127.0.0.1', + ciphers: CIPHERS, + checkServerIdentity: () => {}, + pskCallback: () => ({ + psk: Buffer.alloc(32), + identity: 'test-identity', + }), + }); + + client.on('error', () => {}); + })); +}); + +// Test 2: PSK server callback throwing should emit tlsClientError +tests.push((done) => { + console.log(' - Server pskCallback throws -> tlsClientError'); + const server = tls.createServer({ + ciphers: CIPHERS, + pskCallback: common.mustCall(() => { + console.log(' pskCallback called, throwing error'); + throw new Error('Intentional callback error'); + }), + pskIdentityHint: 'test-hint', + }); + + server.on('tlsClientError', common.mustCall((err, socket) => { + console.log(' tlsClientError received:', err.message); + assert.ok(err instanceof Error); + assert.strictEqual(err.message, 'Intentional callback error'); + socket.destroy(); + console.log(' Test 2 PASSED'); + server.close(done); + })); + + server.on('secureConnection', common.mustNotCall()); + + server.listen(0, common.mustCall(() => { + const client = tls.connect({ + port: server.address().port, + host: '127.0.0.1', + ciphers: CIPHERS, + checkServerIdentity: () => {}, + pskCallback: () => ({ + psk: Buffer.alloc(32), + identity: 'test-identity', + }), + }); + + client.on('error', () => {}); + })); +}); + +// Test 3: PSK client callback returning invalid type should emit error event +tests.push((done) => { + console.log(' - Client pskCallback returns invalid type -> client error'); + const PSK = Buffer.alloc(32); + + const server = tls.createServer({ + ciphers: CIPHERS, + pskCallback: () => PSK, + pskIdentityHint: 'test-hint', + }); + + server.on('secureConnection', common.mustNotCall()); + + server.listen(0, common.mustCall(() => { + const client = tls.connect({ + port: server.address().port, + host: '127.0.0.1', + ciphers: CIPHERS, + checkServerIdentity: () => {}, + pskCallback: common.mustCall(() => { + console.log(' client pskCallback called, returning invalid string'); + // Return invalid type - should cause validation error + return 'invalid-should-be-object'; + }), + }); + + client.on('error', common.mustCall((err) => { + console.log(' client error received:', err.code); + assert.ok(err instanceof Error); + assert.strictEqual(err.code, 'ERR_INVALID_ARG_TYPE'); + console.log(' Test 3 PASSED'); + server.close(done); + })); + })); +}); + +// Test 4: PSK client callback throwing should emit error event +tests.push((done) => { + console.log(' - Client pskCallback throws -> client error'); + const PSK = Buffer.alloc(32); + + const server = tls.createServer({ + ciphers: CIPHERS, + pskCallback: () => PSK, + pskIdentityHint: 'test-hint', + }); + + server.on('secureConnection', common.mustNotCall()); + + server.listen(0, common.mustCall(() => { + const client = tls.connect({ + port: server.address().port, + host: '127.0.0.1', + ciphers: CIPHERS, + checkServerIdentity: () => {}, + pskCallback: common.mustCall(() => { + console.log(' client pskCallback called, throwing error'); + throw new Error('Intentional client PSK callback error'); + }), + }); + + client.on('error', common.mustCall((err) => { + console.log(' client error received:', err.message); + assert.ok(err instanceof Error); + assert.strictEqual(err.message, 'Intentional client PSK callback error'); + console.log(' Test 4 PASSED'); + server.close(done); + })); + })); +}); + +// Start running tests +runNextTest(); From add20cf58b2d54759ea8b0d07640c4da61cee604 Mon Sep 17 00:00:00 2001 From: Erik Olofsson Date: Mon, 19 Jan 2026 13:16:13 +0100 Subject: [PATCH 5/6] CVE-2025-59466 --- src/async_wrap.cc | 9 +- src/node_errors.cc | 97 ++++++++++++++++++- src/node_errors.h | 2 +- ...async-hooks-stack-overflow-nested-async.js | 80 +++++++++++++++ ...st-async-hooks-stack-overflow-try-catch.js | 48 +++++++++ .../test-async-hooks-stack-overflow.js | 48 +++++++++ ...andler-stack-overflow-on-stack-overflow.js | 29 ++++++ ...caught-exception-handler-stack-overflow.js | 30 ++++++ 8 files changed, 336 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-async-hooks-stack-overflow-nested-async.js create mode 100644 test/parallel/test-async-hooks-stack-overflow-try-catch.js create mode 100644 test/parallel/test-async-hooks-stack-overflow.js create mode 100644 test/parallel/test-uncaught-exception-handler-stack-overflow-on-stack-overflow.js create mode 100644 test/parallel/test-uncaught-exception-handler-stack-overflow.js diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 7590ab0197dfd6..ea78287f9cf067 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -105,7 +105,8 @@ struct AsyncWrapObject : public AsyncWrap { void AsyncWrap::DestroyAsyncIdsCallback(Environment* env) { Local fn = env->async_hooks_destroy_function(); - TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal); + TryCatchScope try_catch(env, + TryCatchScope::CatchMode::kFatalRethrowStackOverflow); do { std::vector destroy_async_id_list; @@ -134,7 +135,8 @@ void Emit(Environment* env, double async_id, AsyncHooks::Fields type, HandleScope handle_scope(env->isolate()); Local async_id_value = Number::New(env->isolate(), async_id); - TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal); + TryCatchScope try_catch(env, + TryCatchScope::CatchMode::kFatalRethrowStackOverflow); USE(fn->Call(env->context(), Undefined(env->isolate()), 1, &async_id_value)); } @@ -671,7 +673,8 @@ void AsyncWrap::EmitAsyncInit(Environment* env, object, }; - TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal); + TryCatchScope try_catch(env, + TryCatchScope::CatchMode::kFatalRethrowStackOverflow); USE(init_fn->Call(env->context(), object, arraysize(argv), argv)); } diff --git a/src/node_errors.cc b/src/node_errors.cc index 94e19f70aa288f..b72134f3bb7c26 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -1,3 +1,4 @@ +#include #include #include @@ -27,12 +28,47 @@ using v8::MaybeLocal; using v8::Message; using v8::Object; using v8::ScriptOrigin; +using v8::EscapableHandleScope; using v8::StackFrame; using v8::StackTrace; using v8::String; using v8::Undefined; using v8::Value; +static std::atomic is_in_oom{false}; +static thread_local std::atomic is_retrieving_js_stacktrace{false}; + +MaybeLocal GetCurrentStackTrace(Isolate* isolate, int frame_count) { + if (isolate == nullptr) { + return MaybeLocal(); + } + // Generating JavaScript stack trace can result in V8 fatal error, + // which can re-enter this function. + if (is_retrieving_js_stacktrace.load()) { + return MaybeLocal(); + } + + // Can not capture the stacktrace when the isolate is in a OOM state or no + // context is entered. + if (is_in_oom.load() || !isolate->InContext()) { + return MaybeLocal(); + } + + constexpr StackTrace::StackTraceOptions options = + static_cast( + StackTrace::kDetailed | + StackTrace::kExposeFramesAcrossSecurityOrigins); + + is_retrieving_js_stacktrace.store(true); + EscapableHandleScope scope(isolate); + Local stack = + StackTrace::CurrentStackTrace(isolate, frame_count, options); + + is_retrieving_js_stacktrace.store(false); + + return scope.Escape(stack); +} + bool IsExceptionDecorated(Environment* env, Local er) { if (!er.IsEmpty() && er->IsObject()) { Local err_obj = er.As(); @@ -443,13 +479,52 @@ void OnFatalError(const char* location, const char* message) { ABORT(); } +// Check if an exception is a stack overflow error (RangeError with +// "Maximum call stack size exceeded" message). This is used to handle +// stack overflow specially in TryCatchScope - instead of immediately +// exiting, we can use the red zone to re-throw to user code. +static bool IsStackOverflowError(Isolate* isolate, Local exception) { + if (!exception->IsNativeError()) return false; + + Local err_obj = exception.As(); + Local constructor_name = err_obj->GetConstructorName(); + + // Must be a RangeError + Utf8Value name(isolate, constructor_name); + if (strcmp(*name, "RangeError") != 0) return false; + + // Check for the specific stack overflow message + Local context = isolate->GetCurrentContext(); + Local message_val; + if (!err_obj->Get(context, String::NewFromUtf8Literal(isolate, "message")) + .ToLocal(&message_val)) { + return false; + } + + if (!message_val->IsString()) return false; + + Utf8Value message(isolate, message_val.As()); + return strcmp(*message, "Maximum call stack size exceeded") == 0; +} + namespace errors { TryCatchScope::~TryCatchScope() { - if (HasCaught() && !HasTerminated() && mode_ == CatchMode::kFatal) { + if (HasCaught() && !HasTerminated() && mode_ != CatchMode::kNormal) { HandleScope scope(env_->isolate()); Local exception = Exception(); Local message = Message(); + + // Special handling for stack overflow errors in async_hooks: instead of + // immediately exiting, re-throw the exception. This allows the exception + // to propagate to user code's try-catch blocks. + if (mode_ == CatchMode::kFatalRethrowStackOverflow && + IsStackOverflowError(env_->isolate(), exception)) { + ReThrow(); + Reset(); + return; + } + EnhanceFatalException enhance = CanContinue() ? EnhanceFatalException::kEnhance : EnhanceFatalException::kDontEnhance; if (message.IsEmpty()) @@ -953,8 +1028,24 @@ void TriggerUncaughtException(Isolate* isolate, if (env->can_call_into_js()) { // We do not expect the global uncaught exception itself to throw any more // exceptions. If it does, exit the current Node.js instance. - errors::TryCatchScope try_catch(env, - errors::TryCatchScope::CatchMode::kFatal); + // Special case: if the original error was a stack overflow and calling + // _fatalException causes another stack overflow, rethrow it to allow + // user code's try-catch blocks to potentially catch it. + auto is_stack_overflow = [&] { + return IsStackOverflowError(env->isolate(), error); + }; + // Without a JS stack, rethrowing may or may not do anything. + auto has_js_stack = [&] { + HandleScope handle_scope(env->isolate()); + Local stack; + return GetCurrentStackTrace(env->isolate(), 1).ToLocal(&stack) && + stack->GetFrameCount() > 0; + }; + errors::TryCatchScope::CatchMode mode = + is_stack_overflow() && has_js_stack() + ? errors::TryCatchScope::CatchMode::kFatalRethrowStackOverflow + : errors::TryCatchScope::CatchMode::kFatal; + errors::TryCatchScope try_catch(env, mode); // Explicitly disable verbose exception reporting - // if process._fatalException() throws an error, we don't want it to // trigger the per-isolate message listener which will call this diff --git a/src/node_errors.h b/src/node_errors.h index 1ca93c16ddf790..43126c801485e7 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -195,7 +195,7 @@ namespace errors { class TryCatchScope : public v8::TryCatch { public: - enum class CatchMode { kNormal, kFatal }; + enum class CatchMode { kNormal, kFatal, kFatalRethrowStackOverflow }; explicit TryCatchScope(Environment* env, CatchMode mode = CatchMode::kNormal) : v8::TryCatch(env->isolate()), env_(env), mode_(mode) {} diff --git a/test/parallel/test-async-hooks-stack-overflow-nested-async.js b/test/parallel/test-async-hooks-stack-overflow-nested-async.js new file mode 100644 index 00000000000000..779f8d75ae208f --- /dev/null +++ b/test/parallel/test-async-hooks-stack-overflow-nested-async.js @@ -0,0 +1,80 @@ +'use strict'; + +// This test verifies that stack overflow during deeply nested async operations +// with async_hooks enabled can be caught by try-catch. This simulates real-world +// scenarios like processing deeply nested JSON structures where each level +// creates async operations (e.g., database calls, API requests). + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +if (process.argv[2] === 'child') { + const { createHook } = require('async_hooks'); + + // Enable async_hooks with all callbacks (simulates APM tools) + createHook({ + init() {}, + before() {}, + after() {}, + destroy() {}, + promiseResolve() {}, + }).enable(); + + // Simulate an async operation (like a database call or API request) + async function fetchThing(id) { + return { id, data: `data-${id}` }; + } + + // Recursively process deeply nested data structure + // This will cause stack overflow when the nesting is deep enough + function processData(data, depth = 0) { + if (Array.isArray(data)) { + for (const item of data) { + // Create a promise to trigger async_hooks init callback + fetchThing(depth); + processData(item, depth + 1); + } + } + } + + // Create deeply nested array structure iteratively (to avoid stack overflow + // during creation) + function createNestedArray(depth) { + let result = 'leaf'; + for (let i = 0; i < depth; i++) { + result = [result]; + } + return result; + } + + // Create a very deep nesting that will cause stack overflow during processing + const deeplyNested = createNestedArray(50000); + + try { + processData(deeplyNested); + // Should not complete successfully - the nesting is too deep + console.log('UNEXPECTED: Processing completed without error'); + process.exit(1); + } catch (err) { + assert.strictEqual(err.name, 'RangeError'); + assert.match(err.message, /Maximum call stack size exceeded/); + console.log('SUCCESS: try-catch caught the stack overflow in nested async'); + process.exit(0); + } +} else { + // Parent process - spawn the child and check exit code + const result = spawnSync( + process.execPath, + [__filename, 'child'], + { encoding: 'utf8', timeout: 30000 } + ); + + // Should exit successfully (try-catch worked) + assert.strictEqual(result.status, 0, + `Expected exit code 0, got ${result.status}.\n` + + `stdout: ${result.stdout}\n` + + `stderr: ${result.stderr}`); + // Verify the error was handled by try-catch + assert.match(result.stdout, /SUCCESS: try-catch caught the stack overflow/); +} diff --git a/test/parallel/test-async-hooks-stack-overflow-try-catch.js b/test/parallel/test-async-hooks-stack-overflow-try-catch.js new file mode 100644 index 00000000000000..16d065847c6039 --- /dev/null +++ b/test/parallel/test-async-hooks-stack-overflow-try-catch.js @@ -0,0 +1,48 @@ +'use strict'; + +// This test verifies that when a stack overflow occurs with async_hooks +// enabled, the exception can be caught by try-catch blocks in user code. +// This is a regression test for CVE-2025-59466. + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +if (process.argv[2] === 'child') { + const { createHook } = require('async_hooks'); + + createHook({ init() {} }).enable(); + + function recursive(depth = 0) { + // Create a promise to trigger async_hooks init callback + new Promise(() => {}); + return recursive(depth + 1); + } + + try { + recursive(); + // Should not reach here + process.exit(1); + } catch (err) { + assert.strictEqual(err.name, 'RangeError'); + assert.match(err.message, /Maximum call stack size exceeded/); + console.log('SUCCESS: try-catch caught the stack overflow'); + process.exit(0); + } + + // Should not reach here + process.exit(2); +} else { + // Parent process - spawn the child and check exit code + const result = spawnSync( + process.execPath, + [__filename, 'child'], + { encoding: 'utf8', timeout: 30000 } + ); + + assert.strictEqual(result.status, 0, + `Expected exit code 0 (try-catch worked), got ${result.status}.\n` + + `stdout: ${result.stdout}\n` + + `stderr: ${result.stderr}`); + assert.match(result.stdout, /SUCCESS: try-catch caught the stack overflow/); +} diff --git a/test/parallel/test-async-hooks-stack-overflow.js b/test/parallel/test-async-hooks-stack-overflow.js new file mode 100644 index 00000000000000..70f8288eb56a0e --- /dev/null +++ b/test/parallel/test-async-hooks-stack-overflow.js @@ -0,0 +1,48 @@ +'use strict'; + +// This test verifies that when a stack overflow occurs with async_hooks +// enabled, the uncaughtException handler is still called instead of the +// process crashing with exit code 7. +// This is a regression test for CVE-2025-59466. + +const common = require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +if (process.argv[2] === 'child') { + const { createHook } = require('async_hooks'); + + let handlerCalled = false; + + function recursive() { + // Create a promise to trigger async_hooks init callback + new Promise(() => {}); + return recursive(); + } + + createHook({ init() {} }).enable(); + + process.on('uncaughtException', common.mustCall((err) => { + assert.strictEqual(err.name, 'RangeError'); + assert.match(err.message, /Maximum call stack size exceeded/); + // Ensure handler is only called once + assert.strictEqual(handlerCalled, false); + handlerCalled = true; + })); + + setImmediate(recursive); +} else { + // Parent process - spawn the child and check exit code + const result = spawnSync( + process.execPath, + [__filename, 'child'], + { encoding: 'utf8', timeout: 30000 } + ); + + // Should exit with code 0 (handler was called and handled the exception) + // Previously would exit with code 7 (kExceptionInFatalExceptionHandler) + assert.strictEqual(result.status, 0, + `Expected exit code 0, got ${result.status}.\n` + + `stdout: ${result.stdout}\n` + + `stderr: ${result.stderr}`); +} diff --git a/test/parallel/test-uncaught-exception-handler-stack-overflow-on-stack-overflow.js b/test/parallel/test-uncaught-exception-handler-stack-overflow-on-stack-overflow.js new file mode 100644 index 00000000000000..1923b7f24d995d --- /dev/null +++ b/test/parallel/test-uncaught-exception-handler-stack-overflow-on-stack-overflow.js @@ -0,0 +1,29 @@ +'use strict'; + +// This test verifies that when the uncaughtException handler itself causes +// a stack overflow, the process exits with a non-zero exit code. +// This is important to ensure we don't silently swallow errors. + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +if (process.argv[2] === 'child') { + function f() { f(); } + process.on('uncaughtException', f); + f(); +} else { + // Parent process - spawn the child and check exit code + const result = spawnSync( + process.execPath, + [__filename, 'child'], + { encoding: 'utf8', timeout: 30000 } + ); + + // Should exit with non-zero exit code since the uncaughtException handler + // itself caused a stack overflow. + assert.notStrictEqual(result.status, 0, + `Expected non-zero exit code, got ${result.status}.\n` + + `stdout: ${result.stdout}\n` + + `stderr: ${result.stderr}`); +} diff --git a/test/parallel/test-uncaught-exception-handler-stack-overflow.js b/test/parallel/test-uncaught-exception-handler-stack-overflow.js new file mode 100644 index 00000000000000..9ab940fa9f21d7 --- /dev/null +++ b/test/parallel/test-uncaught-exception-handler-stack-overflow.js @@ -0,0 +1,30 @@ +'use strict'; + +// This test verifies that when the uncaughtException handler itself causes +// a stack overflow, the process exits with a non-zero exit code. +// This is important to ensure we don't silently swallow errors. +// Part of CVE-2025-59466 fix validation. + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +if (process.argv[2] === 'child') { + function f() { f(); } + process.on('uncaughtException', f); + throw new Error('X'); +} else { + // Parent process - spawn the child and check exit code + const result = spawnSync( + process.execPath, + [__filename, 'child'], + { encoding: 'utf8', timeout: 30000 } + ); + + // Should exit with non-zero exit code since the uncaughtException handler + // itself caused a stack overflow. + assert.notStrictEqual(result.status, 0, + `Expected non-zero exit code, got ${result.status}.\n` + + `stdout: ${result.stdout}\n` + + `stderr: ${result.stderr}`); +} From 20d90d45f27ab4fa3d3ab083a839a299e9e31fcc Mon Sep 17 00:00:00 2001 From: Erik Olofsson Date: Mon, 19 Jan 2026 14:32:28 +0100 Subject: [PATCH 6/6] CVE-2025-55131 --- deps/v8/include/v8.h | 7 ++ deps/v8/src/api/api.cc | 17 +++++ lib/buffer.js | 19 ++--- src/node_buffer.cc | 72 ++++++++++++------ ...-buffer-alloc-unsafe-no-zerofill-toggle.js | 74 +++++++++++++++++++ 5 files changed, 154 insertions(+), 35 deletions(-) create mode 100644 test/parallel/test-buffer-alloc-unsafe-no-zerofill-toggle.js diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h index 92d3623df54506..db9a0e8bbff2ee 100644 --- a/deps/v8/include/v8.h +++ b/deps/v8/include/v8.h @@ -5203,6 +5203,13 @@ class V8_EXPORT ArrayBuffer : public Object { */ static std::unique_ptr NewBackingStore(Isolate* isolate, size_t byte_length); + /** + * Returns a new standalone BackingStore with uninitialized memory and + * return nullptr on failure. + * This variant is for not breaking ABI on Node.js LTS. DO NOT USE. + */ + static std::unique_ptr NewBackingStoreForNodeLTS( + Isolate* isolate, size_t byte_length); /** * Returns a new standalone BackingStore that takes over the ownership of * the given buffer. The destructor of the BackingStore invokes the given diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index b462697bc3dd16..9279719ad3f174 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -7535,6 +7535,23 @@ std::unique_ptr v8::ArrayBuffer::NewBackingStore( static_cast(backing_store.release())); } +std::unique_ptr v8::ArrayBuffer::NewBackingStoreForNodeLTS( + Isolate* isolate, size_t byte_length) { + i::Isolate* i_isolate = reinterpret_cast(isolate); + LOG_API(i_isolate, ArrayBuffer, NewBackingStore); + CHECK_LE(byte_length, i::JSArrayBuffer::kMaxByteLength); + ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate); + std::unique_ptr backing_store = + i::BackingStore::Allocate(i_isolate, byte_length, + i::SharedFlag::kNotShared, + i::InitializedFlag::kUninitialized); + if (!backing_store) { + return nullptr; + } + return std::unique_ptr( + static_cast(backing_store.release())); +} + std::unique_ptr v8::ArrayBuffer::NewBackingStore( void* data, size_t byte_length, v8::BackingStore::DeleterCallback deleter, void* deleter_data) { diff --git a/lib/buffer.js b/lib/buffer.js index 757399e5a6465c..3247a9800a5bfb 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -55,6 +55,7 @@ const { compare: _compare, compareOffset, createFromString, + createUnsafeArrayBuffer, fill: bindingFill, indexOfBuffer, indexOfNumber, @@ -64,7 +65,6 @@ const { swap64: _swap64, kMaxLength, kStringMaxLength, - zeroFill: bindingZeroFill } = internalBinding('buffer'); const { getOwnNonIndexProperties, @@ -140,23 +140,20 @@ const constants = ObjectDefineProperties({}, { Buffer.poolSize = 8 * 1024; let poolSize, poolOffset, allocPool; -// A toggle used to access the zero fill setting of the array buffer allocator -// in C++. -// |zeroFill| can be undefined when running inside an isolate where we -// do not own the ArrayBuffer allocator. Zero fill is always on in that case. -const zeroFill = bindingZeroFill || [0]; - const encodingsMap = ObjectCreate(null); for (let i = 0; i < encodings.length; ++i) encodingsMap[encodings[i]] = i; +// CVE-2025-55131: The previous implementation used a zeroFill toggle that was +// vulnerable to race conditions. Now we use a dedicated C++ function to create +// ArrayBuffers with uninitialized memory, bypassing the toggle mechanism entirely. function createUnsafeBuffer(size) { - zeroFill[0] = 0; - try { + // Small allocations (<=64 bytes) are heap-allocated and don't benefit from + // skipping zero-fill, so just use the normal path for simplicity. + if (size <= 64) { return new FastBuffer(size); - } finally { - zeroFill[0] = 1; } + return new FastBuffer(createUnsafeArrayBuffer(size)); } function createPool() { diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 6a66bd833ae345..5d014706138bf8 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -32,8 +32,9 @@ #include "util-inl.h" #include "v8.h" -#include #include +#include +#include #define THROW_AND_RETURN_UNLESS_BUFFER(env, obj) \ THROW_AND_RETURN_IF_NOT_BUFFER(env, obj, "argument") \ @@ -1129,6 +1130,50 @@ void SetBufferPrototype(const FunctionCallbackInfo& args) { } +// Converts a number parameter to size_t suitable for ArrayBuffer sizes +// Could be larger than uint32_t +// See v8::internal::TryNumberToSize and v8::internal::NumberToSize +inline size_t CheckNumberToSize(Local number) { + CHECK(number->IsNumber()); + double value = number.As()->Value(); + // See v8::internal::TryNumberToSize on this (and on < comparison) + double maxSize = static_cast(std::numeric_limits::max()); + CHECK(value >= 0 && value < maxSize); + return static_cast(value); +} + +// Creates an ArrayBuffer with uninitialized memory, bypassing the zero-fill +// toggle mechanism. This is used by Buffer.allocUnsafe and Buffer.allocUnsafeSlow. +// CVE-2025-55131: The zero-fill toggle mechanism was vulnerable to race conditions. +void CreateUnsafeArrayBuffer(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + if (args.Length() != 1) { + return env->ThrowRangeError("Invalid array buffer length"); + } + + size_t size = CheckNumberToSize(args[0]); + + Isolate* isolate = env->isolate(); + + Local buf; + + NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator(); + // 0-length, or zero-fill flag is set, or no allocator available + if (size == 0 || per_process::cli_options->zero_fill_all_buffers || + allocator == nullptr) { + buf = ArrayBuffer::New(isolate, size); + } else { + std::unique_ptr store = + ArrayBuffer::NewBackingStoreForNodeLTS(isolate, size); + if (!store) { + return env->ThrowRangeError("Array buffer allocation failed"); + } + buf = ArrayBuffer::New(isolate, std::move(store)); + } + + args.GetReturnValue().Set(buf); +} + void Initialize(Local target, Local unused, Local context, @@ -1136,6 +1181,8 @@ void Initialize(Local target, Environment* env = Environment::GetCurrent(context); env->SetMethod(target, "setBufferPrototype", SetBufferPrototype); + env->SetMethodNoSideEffect(target, "createUnsafeArrayBuffer", + CreateUnsafeArrayBuffer); env->SetMethodNoSideEffect(target, "createFromString", CreateFromString); env->SetMethodNoSideEffect(target, "byteLengthUtf8", ByteLengthUtf8); @@ -1179,29 +1226,6 @@ void Initialize(Local target, env->SetMethod(target, "utf8Write", StringWrite); Blob::Initialize(env, target); - - // It can be a nullptr when running inside an isolate where we - // do not own the ArrayBuffer allocator. - if (NodeArrayBufferAllocator* allocator = - env->isolate_data()->node_allocator()) { - uint32_t* zero_fill_field = allocator->zero_fill_field(); - std::unique_ptr backing = - ArrayBuffer::NewBackingStore(zero_fill_field, - sizeof(*zero_fill_field), - [](void*, size_t, void*){}, - nullptr); - Local array_buffer = - ArrayBuffer::New(env->isolate(), std::move(backing)); - array_buffer->SetPrivate( - env->context(), - env->untransferable_object_private_symbol(), - True(env->isolate())).Check(); - CHECK(target - ->Set(env->context(), - FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill"), - Uint32Array::New(array_buffer, 0, 1)) - .FromJust()); - } } } // anonymous namespace diff --git a/test/parallel/test-buffer-alloc-unsafe-no-zerofill-toggle.js b/test/parallel/test-buffer-alloc-unsafe-no-zerofill-toggle.js new file mode 100644 index 00000000000000..32c5ba95280588 --- /dev/null +++ b/test/parallel/test-buffer-alloc-unsafe-no-zerofill-toggle.js @@ -0,0 +1,74 @@ +'use strict'; + +// This test verifies that Buffer.allocUnsafe and Buffer.allocUnsafeSlow +// work correctly using the new createUnsafeArrayBuffer C++ function +// instead of the vulnerable zeroFill toggle mechanism. +// This is a regression test for CVE-2025-55131. + +const common = require('../common'); +const assert = require('assert'); + +// Test that Buffer.allocUnsafe works for various sizes +{ + // Small buffer (<=64 bytes, uses heap allocation path) + const small = Buffer.allocUnsafe(32); + assert.strictEqual(small.length, 32); + assert.ok(Buffer.isBuffer(small)); + + // Medium buffer (uses the C++ createUnsafeArrayBuffer) + const medium = Buffer.allocUnsafe(1024); + assert.strictEqual(medium.length, 1024); + assert.ok(Buffer.isBuffer(medium)); + + // Large buffer + const large = Buffer.allocUnsafe(1024 * 1024); + assert.strictEqual(large.length, 1024 * 1024); + assert.ok(Buffer.isBuffer(large)); +} + +// Test that Buffer.allocUnsafeSlow works +{ + const slow = Buffer.allocUnsafeSlow(1024); + assert.strictEqual(slow.length, 1024); + assert.ok(Buffer.isBuffer(slow)); +} + +// Test that zero-length buffers work +{ + const zero = Buffer.allocUnsafe(0); + assert.strictEqual(zero.length, 0); + assert.ok(Buffer.isBuffer(zero)); +} + +// Test that buffers are writable +{ + const buf = Buffer.allocUnsafe(100); + buf.fill(0x42); + for (let i = 0; i < buf.length; i++) { + assert.strictEqual(buf[i], 0x42); + } +} + +// Test that Buffer.alloc (safe version) still zero-fills +{ + const safe = Buffer.alloc(100); + for (let i = 0; i < safe.length; i++) { + assert.strictEqual(safe[i], 0, `Expected 0 at index ${i}, got ${safe[i]}`); + } +} + +// Test pool allocation (uses createUnsafeBuffer internally) +{ + // Multiple small allocations from pool + const bufs = []; + for (let i = 0; i < 100; i++) { + bufs.push(Buffer.allocUnsafe(100)); + } + // Verify all buffers are valid + for (const buf of bufs) { + assert.strictEqual(buf.length, 100); + assert.ok(Buffer.isBuffer(buf)); + } +} + +console.log('All CVE-2025-55131 tests passed');