From 34e83027e16283175b8a7bdbab439604e53c4412 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Mon, 15 Jun 2026 10:48:09 -0700 Subject: [PATCH 1/2] Node-API (Chakra): fix heap corruption when an ObjectWrap constructor throws When a napi ObjectWrap subclass constructor throws (e.g. TextDecoder given an unsupported encoding), the C++ instance is destroyed during stack unwinding, but the wrap finalizer registered by napi_wrap() stays attached to the JS `this` object. A later GC then runs the finalizer on the freed native instance -> use-after-free / double free, surfacing as non-deterministic heap corruption (STATUS_HEAP_CORRUPTION 0xC0000374 / access violation) crashes far from the throw site. Root cause: the addon-api ~ObjectWrap() is supposed to detach the wrap on construction failure, but it calls napi_get_reference_value() on the wrap's weak (refcount 0) reference to obtain `this`, and the Chakra backend returns null for every refcount-0 reference (it cannot track weak-reference liveness with the in-box jsrt API). So ~ObjectWrap() sees an empty object and skips napi_remove_wrap(), leaving the dangling finalizer attached. Fix: in the Chakra construct-call trampoline, if the callback leaves a pending JS exception, detach any wrap left on `this` via napi_remove_wrap() (preserving the pending exception) so the finalizer cannot run on the freed instance. Also guard napi_remove_wrap() against a null `result` out-param (it is now called with nullptr, matching upstream which guards this). Adds a regression test that throws from a wrapped constructor many times and then exercises the heap; without the fix this reliably corrupts the heap on Chakra (Win32 x64/x86). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Core/Node-API/Source/js_native_api_chakra.cc | 27 ++++++++++++++++++-- Tests/UnitTests/Scripts/tests.ts | 14 ++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/Core/Node-API/Source/js_native_api_chakra.cc b/Core/Node-API/Source/js_native_api_chakra.cc index 297be2aa..67066d92 100644 --- a/Core/Node-API/Source/js_native_api_chakra.cc +++ b/Core/Node-API/Source/js_native_api_chakra.cc @@ -172,6 +172,27 @@ class ExternalCallback { napi_value result = externalCallback->_cb( externalCallback->_env, reinterpret_cast(&cbInfo)); + + // If a constructor (construct call) left a pending JS exception, the C++ + // ObjectWrap instance was already destroyed by stack unwinding inside the + // callback, but the wrap finalizer registered by napi_wrap() is still + // attached to `this`. The addon-api ~ObjectWrap() cannot detach it here + // because napi_get_reference_value() returns null for the wrap's weak + // (refcount 0) reference. Detach the wrap now so a later GC does not run + // the finalizer on the freed native instance (use-after-free / heap + // corruption). The pending exception is preserved across the cleanup. + if (isConstructCall) { + bool hasException = false; + if (JsHasException(&hasException) == JsNoError && hasException) { + JsValueRef exception = JS_INVALID_REFERENCE; + if (JsGetAndClearException(&exception) == JsNoError) { + napi_remove_wrap(externalCallback->_env, + reinterpret_cast(arguments[0]), nullptr); + JsSetException(exception); + } + } + } + return reinterpret_cast(result); } @@ -1748,9 +1769,11 @@ napi_status napi_remove_wrap(napi_env env, napi_value js_object, void** result) CHECK_JSRT(env, JsSetExternalData(wrapper, nullptr)); if (externalData != nullptr) { - *result = externalData->Data(); + if (result != nullptr) { + *result = externalData->Data(); + } delete externalData; - } else { + } else if (result != nullptr) { *result = nullptr; } diff --git a/Tests/UnitTests/Scripts/tests.ts b/Tests/UnitTests/Scripts/tests.ts index e0647092..0e77de8a 100644 --- a/Tests/UnitTests/Scripts/tests.ts +++ b/Tests/UnitTests/Scripts/tests.ts @@ -1490,6 +1490,20 @@ describe("TextDecoder", function () { expect(result).to.equal("H\0i"); expect(result.length).to.equal(3); }); + + it("throwing from the constructor repeatedly does not corrupt native state", function () { + // Regression for a ChakraCore N-API ObjectWrap bug: when a wrapped + // constructor throws, the native instance is destroyed during stack + // unwinding but the wrap finalizer stayed attached to `this`, so a + // later GC ran the finalizer on freed memory (heap corruption). Throw + // many times to create many dangling wraps, then allocate/decode to + // exercise the heap and surface any corruption within this test run. + for (let i = 0; i < 100; ++i) { + expect(() => new TextDecoder("utf-16")).to.throw(); + } + const decoder = new TextDecoder("utf-8"); + expect(decoder.decode(new Uint8Array([79, 75]))).to.equal("OK"); + }); }); describe("TextEncoder", function () { From ff58d2136423e432aba621e397c8995d186c65ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Wed, 17 Jun 2026 16:47:35 -0700 Subject: [PATCH 2/2] Update Tests/UnitTests/Scripts/tests.ts Co-authored-by: Gary Hsu --- Tests/UnitTests/Scripts/tests.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/UnitTests/Scripts/tests.ts b/Tests/UnitTests/Scripts/tests.ts index 0e77de8a..ff2d7915 100644 --- a/Tests/UnitTests/Scripts/tests.ts +++ b/Tests/UnitTests/Scripts/tests.ts @@ -1492,7 +1492,7 @@ describe("TextDecoder", function () { }); it("throwing from the constructor repeatedly does not corrupt native state", function () { - // Regression for a ChakraCore N-API ObjectWrap bug: when a wrapped + // Regression for a Chakra N-API ObjectWrap bug: when a wrapped // constructor throws, the native instance is destroyed during stack // unwinding but the wrap finalizer stayed attached to `this`, so a // later GC ran the finalizer on freed memory (heap corruption). Throw