Skip to content

Commit d7a5c58

Browse files
mcollinaRafaelGSS
authored andcommitted
src: rethrow stack overflow exceptions in async_hooks
When a stack overflow exception occurs during async_hooks callbacks (which use TryCatchScope::kFatal), detect the specific "Maximum call stack size exceeded" RangeError and re-throw it instead of immediately calling FatalException. This allows user code to catch the exception with try-catch blocks instead of requiring uncaughtException handlers. The implementation adds IsStackOverflowError() helper to detect stack overflow RangeErrors and re-throws them in TryCatchScope destructor instead of calling FatalException. This fixes the issue where async_hooks would cause stack overflow exceptions to exit with code 7 (kExceptionInFatalExceptionHandler) instead of being catchable. Fixes: #37989 Ref: https://hackerone.com/reports/3456295 PR-URL: nodejs-private/node-private#773 Refs: https://hackerone.com/reports/3456295 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> CVE-ID: CVE-2025-59466
1 parent 8f9ba3f commit d7a5c58

10 files changed

+306
-14
lines changed

src/async_wrap.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ static const char* const provider_names[] = {
6767
void AsyncWrap::DestroyAsyncIdsCallback(Environment* env) {
6868
Local<Function> fn = env->async_hooks_destroy_function();
6969

70-
TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal);
70+
TryCatchScope try_catch(env,
71+
TryCatchScope::CatchMode::kFatalRethrowStackOverflow);
7172

7273
do {
7374
std::vector<double> destroy_async_id_list;
@@ -96,7 +97,8 @@ void Emit(Environment* env, double async_id, AsyncHooks::Fields type,
9697

9798
HandleScope handle_scope(env->isolate());
9899
Local<Value> async_id_value = Number::New(env->isolate(), async_id);
99-
TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal);
100+
TryCatchScope try_catch(env,
101+
TryCatchScope::CatchMode::kFatalRethrowStackOverflow);
100102
USE(fn->Call(env->context(), Undefined(env->isolate()), 1, &async_id_value));
101103
}
102104

@@ -668,7 +670,8 @@ void AsyncWrap::EmitAsyncInit(Environment* env,
668670
object,
669671
};
670672

671-
TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal);
673+
TryCatchScope try_catch(env,
674+
TryCatchScope::CatchMode::kFatalRethrowStackOverflow);
672675
USE(init_fn->Call(env->context(), object, arraysize(argv), argv));
673676
}
674677

src/debug_utils.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,8 @@ void DumpJavaScriptBacktrace(FILE* fp) {
333333
}
334334

335335
Local<StackTrace> stack;
336-
if (!GetCurrentStackTrace(isolate).ToLocal(&stack)) {
336+
if (!GetCurrentStackTrace(isolate).ToLocal(&stack) ||
337+
stack->GetFrameCount() == 0) {
337338
return;
338339
}
339340

src/node_errors.cc

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ static std::string GetErrorSource(Isolate* isolate,
193193
}
194194

195195
static std::atomic<bool> is_in_oom{false};
196-
static std::atomic<bool> is_retrieving_js_stacktrace{false};
196+
static thread_local std::atomic<bool> is_retrieving_js_stacktrace{false};
197197
MaybeLocal<StackTrace> GetCurrentStackTrace(Isolate* isolate, int frame_count) {
198198
if (isolate == nullptr) {
199199
return MaybeLocal<StackTrace>();
@@ -221,9 +221,6 @@ MaybeLocal<StackTrace> GetCurrentStackTrace(Isolate* isolate, int frame_count) {
221221
StackTrace::CurrentStackTrace(isolate, frame_count, options);
222222

223223
is_retrieving_js_stacktrace.store(false);
224-
if (stack->GetFrameCount() == 0) {
225-
return MaybeLocal<StackTrace>();
226-
}
227224

228225
return scope.Escape(stack);
229226
}
@@ -298,7 +295,8 @@ void PrintStackTrace(Isolate* isolate,
298295

299296
void PrintCurrentStackTrace(Isolate* isolate, StackTracePrefix prefix) {
300297
Local<StackTrace> stack;
301-
if (GetCurrentStackTrace(isolate).ToLocal(&stack)) {
298+
if (GetCurrentStackTrace(isolate).ToLocal(&stack) &&
299+
stack->GetFrameCount() > 0) {
302300
PrintStackTrace(isolate, stack, prefix);
303301
}
304302
}
@@ -669,13 +667,52 @@ v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings(
669667
};
670668
}
671669

670+
// Check if an exception is a stack overflow error (RangeError with
671+
// "Maximum call stack size exceeded" message). This is used to handle
672+
// stack overflow specially in TryCatchScope - instead of immediately
673+
// exiting, we can use the red zone to re-throw to user code.
674+
static bool IsStackOverflowError(Isolate* isolate, Local<Value> exception) {
675+
if (!exception->IsNativeError()) return false;
676+
677+
Local<Object> err_obj = exception.As<Object>();
678+
Local<String> constructor_name = err_obj->GetConstructorName();
679+
680+
// Must be a RangeError
681+
Utf8Value name(isolate, constructor_name);
682+
if (name.ToStringView() != "RangeError") return false;
683+
684+
// Check for the specific stack overflow message
685+
Local<Context> context = isolate->GetCurrentContext();
686+
Local<Value> message_val;
687+
if (!err_obj->Get(context, String::NewFromUtf8Literal(isolate, "message"))
688+
.ToLocal(&message_val)) {
689+
return false;
690+
}
691+
692+
if (!message_val->IsString()) return false;
693+
694+
Utf8Value message(isolate, message_val.As<String>());
695+
return message.ToStringView() == "Maximum call stack size exceeded";
696+
}
697+
672698
namespace errors {
673699

674700
TryCatchScope::~TryCatchScope() {
675-
if (HasCaught() && !HasTerminated() && mode_ == CatchMode::kFatal) {
701+
if (HasCaught() && !HasTerminated() && mode_ != CatchMode::kNormal) {
676702
HandleScope scope(env_->isolate());
677703
Local<v8::Value> exception = Exception();
678704
Local<v8::Message> message = Message();
705+
706+
// Special handling for stack overflow errors in async_hooks: instead of
707+
// immediately exiting, re-throw the exception. This allows the exception
708+
// to propagate to user code's try-catch blocks.
709+
if (mode_ == CatchMode::kFatalRethrowStackOverflow &&
710+
IsStackOverflowError(env_->isolate(), exception)) {
711+
ReThrow();
712+
Reset();
713+
return;
714+
}
715+
679716
EnhanceFatalException enhance = CanContinue() ?
680717
EnhanceFatalException::kEnhance : EnhanceFatalException::kDontEnhance;
681718
if (message.IsEmpty())
@@ -1230,8 +1267,26 @@ void TriggerUncaughtException(Isolate* isolate,
12301267
if (env->can_call_into_js()) {
12311268
// We do not expect the global uncaught exception itself to throw any more
12321269
// exceptions. If it does, exit the current Node.js instance.
1233-
errors::TryCatchScope try_catch(env,
1234-
errors::TryCatchScope::CatchMode::kFatal);
1270+
// Special case: if the original error was a stack overflow and calling
1271+
// _fatalException causes another stack overflow, rethrow it to allow
1272+
// user code's try-catch blocks to potentially catch it.
1273+
auto is_stack_overflow = [&] {
1274+
return IsStackOverflowError(env->isolate(), error);
1275+
};
1276+
// Without a JS stack, rethrowing may or may not do anything.
1277+
// TODO(addaleax): In V8, expose a way to check whether there is a JS stack
1278+
// or TryCatch that would capture the rethrown exception.
1279+
auto has_js_stack = [&] {
1280+
HandleScope handle_scope(env->isolate());
1281+
Local<StackTrace> stack;
1282+
return GetCurrentStackTrace(env->isolate(), 1).ToLocal(&stack) &&
1283+
stack->GetFrameCount() > 0;
1284+
};
1285+
errors::TryCatchScope::CatchMode mode =
1286+
is_stack_overflow() && has_js_stack()
1287+
? errors::TryCatchScope::CatchMode::kFatalRethrowStackOverflow
1288+
: errors::TryCatchScope::CatchMode::kFatal;
1289+
errors::TryCatchScope try_catch(env, mode);
12351290
// Explicitly disable verbose exception reporting -
12361291
// if process._fatalException() throws an error, we don't want it to
12371292
// trigger the per-isolate message listener which will call this

src/node_errors.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ namespace errors {
293293

294294
class TryCatchScope : public v8::TryCatch {
295295
public:
296-
enum class CatchMode { kNormal, kFatal };
296+
enum class CatchMode { kNormal, kFatal, kFatalRethrowStackOverflow };
297297

298298
explicit TryCatchScope(Environment* env, CatchMode mode = CatchMode::kNormal)
299299
: v8::TryCatch(env->isolate()), env_(env), mode_(mode) {}

src/node_report.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,8 @@ static void PrintJavaScriptStack(JSONWriter* writer,
465465
const char* trigger) {
466466
HandleScope scope(isolate);
467467
Local<v8::StackTrace> stack;
468-
if (!GetCurrentStackTrace(isolate, MAX_FRAME_COUNT).ToLocal(&stack)) {
468+
if (!GetCurrentStackTrace(isolate, MAX_FRAME_COUNT).ToLocal(&stack) ||
469+
stack->GetFrameCount() == 0) {
469470
PrintEmptyJavaScriptStack(writer);
470471
return;
471472
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
'use strict';
2+
3+
// This test verifies that stack overflow during deeply nested async operations
4+
// with async_hooks enabled can be caught by try-catch. This simulates real-world
5+
// scenarios like processing deeply nested JSON structures where each level
6+
// creates async operations (e.g., database calls, API requests).
7+
8+
require('../common');
9+
const assert = require('assert');
10+
const { spawnSync } = require('child_process');
11+
12+
if (process.argv[2] === 'child') {
13+
const { createHook } = require('async_hooks');
14+
15+
// Enable async_hooks with all callbacks (simulates APM tools)
16+
createHook({
17+
init() {},
18+
before() {},
19+
after() {},
20+
destroy() {},
21+
promiseResolve() {},
22+
}).enable();
23+
24+
// Simulate an async operation (like a database call or API request)
25+
async function fetchThing(id) {
26+
return { id, data: `data-${id}` };
27+
}
28+
29+
// Recursively process deeply nested data structure
30+
// This will cause stack overflow when the nesting is deep enough
31+
function processData(data, depth = 0) {
32+
if (Array.isArray(data)) {
33+
for (const item of data) {
34+
// Create a promise to trigger async_hooks init callback
35+
fetchThing(depth);
36+
processData(item, depth + 1);
37+
}
38+
}
39+
}
40+
41+
// Create deeply nested array structure iteratively (to avoid stack overflow
42+
// during creation)
43+
function createNestedArray(depth) {
44+
let result = 'leaf';
45+
for (let i = 0; i < depth; i++) {
46+
result = [result];
47+
}
48+
return result;
49+
}
50+
51+
// Create a very deep nesting that will cause stack overflow during processing
52+
const deeplyNested = createNestedArray(50000);
53+
54+
try {
55+
processData(deeplyNested);
56+
// Should not complete successfully - the nesting is too deep
57+
console.log('UNEXPECTED: Processing completed without error');
58+
process.exit(1);
59+
} catch (err) {
60+
assert.strictEqual(err.name, 'RangeError');
61+
assert.match(err.message, /Maximum call stack size exceeded/);
62+
console.log('SUCCESS: try-catch caught the stack overflow in nested async');
63+
process.exit(0);
64+
}
65+
} else {
66+
// Parent process - spawn the child and check exit code
67+
const result = spawnSync(
68+
process.execPath,
69+
[__filename, 'child'],
70+
{ encoding: 'utf8', timeout: 30000 }
71+
);
72+
73+
// Should exit successfully (try-catch worked)
74+
assert.strictEqual(result.status, 0,
75+
`Expected exit code 0, got ${result.status}.\n` +
76+
`stdout: ${result.stdout}\n` +
77+
`stderr: ${result.stderr}`);
78+
// Verify the error was handled by try-catch
79+
assert.match(result.stdout, /SUCCESS: try-catch caught the stack overflow/);
80+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
'use strict';
2+
3+
// This test verifies that when a stack overflow occurs with async_hooks
4+
// enabled, the exception can be caught by try-catch blocks in user code.
5+
6+
require('../common');
7+
const assert = require('assert');
8+
const { spawnSync } = require('child_process');
9+
10+
if (process.argv[2] === 'child') {
11+
const { createHook } = require('async_hooks');
12+
13+
createHook({ init() {} }).enable();
14+
15+
function recursive(depth = 0) {
16+
// Create a promise to trigger async_hooks init callback
17+
new Promise(() => {});
18+
return recursive(depth + 1);
19+
}
20+
21+
try {
22+
recursive();
23+
// Should not reach here
24+
process.exit(1);
25+
} catch (err) {
26+
assert.strictEqual(err.name, 'RangeError');
27+
assert.match(err.message, /Maximum call stack size exceeded/);
28+
console.log('SUCCESS: try-catch caught the stack overflow');
29+
process.exit(0);
30+
}
31+
32+
// Should not reach here
33+
process.exit(2);
34+
} else {
35+
// Parent process - spawn the child and check exit code
36+
const result = spawnSync(
37+
process.execPath,
38+
[__filename, 'child'],
39+
{ encoding: 'utf8', timeout: 30000 }
40+
);
41+
42+
assert.strictEqual(result.status, 0,
43+
`Expected exit code 0 (try-catch worked), got ${result.status}.\n` +
44+
`stdout: ${result.stdout}\n` +
45+
`stderr: ${result.stderr}`);
46+
assert.match(result.stdout, /SUCCESS: try-catch caught the stack overflow/);
47+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
'use strict';
2+
3+
// This test verifies that when a stack overflow occurs with async_hooks
4+
// enabled, the uncaughtException handler is still called instead of the
5+
// process crashing with exit code 7.
6+
7+
const common = require('../common');
8+
const assert = require('assert');
9+
const { spawnSync } = require('child_process');
10+
11+
if (process.argv[2] === 'child') {
12+
const { createHook } = require('async_hooks');
13+
14+
let handlerCalled = false;
15+
16+
function recursive() {
17+
// Create a promise to trigger async_hooks init callback
18+
new Promise(() => {});
19+
return recursive();
20+
}
21+
22+
createHook({ init() {} }).enable();
23+
24+
process.on('uncaughtException', common.mustCall((err) => {
25+
assert.strictEqual(err.name, 'RangeError');
26+
assert.match(err.message, /Maximum call stack size exceeded/);
27+
// Ensure handler is only called once
28+
assert.strictEqual(handlerCalled, false);
29+
handlerCalled = true;
30+
}));
31+
32+
setImmediate(recursive);
33+
} else {
34+
// Parent process - spawn the child and check exit code
35+
const result = spawnSync(
36+
process.execPath,
37+
[__filename, 'child'],
38+
{ encoding: 'utf8', timeout: 30000 }
39+
);
40+
41+
// Should exit with code 0 (handler was called and handled the exception)
42+
// Previously would exit with code 7 (kExceptionInFatalExceptionHandler)
43+
assert.strictEqual(result.status, 0,
44+
`Expected exit code 0, got ${result.status}.\n` +
45+
`stdout: ${result.stdout}\n` +
46+
`stderr: ${result.stderr}`);
47+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
'use strict';
2+
3+
// This test verifies that when the uncaughtException handler itself causes
4+
// a stack overflow, the process exits with a non-zero exit code.
5+
// This is important to ensure we don't silently swallow errors.
6+
7+
require('../common');
8+
const assert = require('assert');
9+
const { spawnSync } = require('child_process');
10+
11+
if (process.argv[2] === 'child') {
12+
function f() { f(); }
13+
process.on('uncaughtException', f);
14+
f();
15+
} else {
16+
// Parent process - spawn the child and check exit code
17+
const result = spawnSync(
18+
process.execPath,
19+
[__filename, 'child'],
20+
{ encoding: 'utf8', timeout: 30000 }
21+
);
22+
23+
// Should exit with non-zero exit code since the uncaughtException handler
24+
// itself caused a stack overflow.
25+
assert.notStrictEqual(result.status, 0,
26+
`Expected non-zero exit code, got ${result.status}.\n` +
27+
`stdout: ${result.stdout}\n` +
28+
`stderr: ${result.stderr}`);
29+
}

0 commit comments

Comments
 (0)