From ea58ba58d8226e594376aed65fca6aab88cc1710 Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 11 Apr 2026 22:31:37 +0000 Subject: [PATCH 1/3] Fix use-after-free in Worker::terminate() and assertion in Reflect.construct(mock()) Worker: the Zig WebWorker struct is freed on the worker thread when it exits, but the C++ Worker kept a raw impl_ pointer to it. A subsequent worker.terminate()/ref()/unref() from JS would dereference freed memory. Guard impl_ with a lock and clear it before the Zig side frees the struct. mock(): jsMockFunctionCall was registered as both the call and construct handler and can return undefined. Interpreter::executeConstruct calls asObject() on the result of a native constructor, which asserts isCell(). Add a dedicated construct wrapper that falls back to an empty object when the call path returns a non-object. --- src/bun.js/bindings/JSMockFunction.cpp | 15 +++++- src/bun.js/bindings/webcore/Worker.cpp | 17 ++++++- src/bun.js/bindings/webcore/Worker.h | 2 + test/js/bun/test/mock-construct.test.ts | 47 +++++++++++++++++++ .../worker-terminate-after-exit.test.ts | 35 ++++++++++++++ 5 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 test/js/bun/test/mock-construct.test.ts create mode 100644 test/js/web/workers/worker-terminate-after-exit.test.ts diff --git a/src/bun.js/bindings/JSMockFunction.cpp b/src/bun.js/bindings/JSMockFunction.cpp index 1c760357186a..c1e8ff2d5dd2 100644 --- a/src/bun.js/bindings/JSMockFunction.cpp +++ b/src/bun.js/bindings/JSMockFunction.cpp @@ -86,6 +86,7 @@ inline To tryJSDynamicCast(JSC::WriteBarrier& from) } JSC_DECLARE_HOST_FUNCTION(jsMockFunctionCall); +JSC_DECLARE_HOST_FUNCTION(jsMockFunctionConstruct); JSC_DECLARE_CUSTOM_GETTER(jsMockFunctionGetter_protoImpl); JSC_DECLARE_CUSTOM_GETTER(jsMockFunctionGetter_mock); JSC_DECLARE_HOST_FUNCTION(jsMockFunctionGetter_mockGetLastCall); @@ -462,7 +463,7 @@ class JSMockFunction : public JSC::InternalFunction { } JSMockFunction(JSC::VM& vm, JSC::Structure* structure, CallbackKind wrapKind) - : Base(vm, structure, jsMockFunctionCall, jsMockFunctionCall) + : Base(vm, structure, jsMockFunctionCall, jsMockFunctionConstruct) { initMock(); } @@ -981,6 +982,18 @@ JSC_DEFINE_HOST_FUNCTION(jsMockFunctionCall, (JSGlobalObject * lexicalGlobalObje return JSValue::encode(jsUndefined()); } +JSC_DEFINE_HOST_FUNCTION(jsMockFunctionConstruct, (JSGlobalObject * lexicalGlobalObject, CallFrame* callframe)) +{ + auto& vm = JSC::getVM(lexicalGlobalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + JSC::EncodedJSValue encodedResult = jsMockFunctionCall(lexicalGlobalObject, callframe); + RETURN_IF_EXCEPTION(scope, {}); + JSValue result = JSValue::decode(encodedResult); + if (!result.isObject()) + return JSValue::encode(JSC::constructEmptyObject(lexicalGlobalObject)); + return encodedResult; +} + void JSMockFunctionPrototype::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject) { Base::finishCreation(vm); diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index 8db7b462b33a..60782fc937e5 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -133,12 +133,21 @@ extern "C" void WebWorker__setRef( void Worker::setKeepAlive(bool keepAlive) { - WebWorker__setRef(impl_, keepAlive); + Locker locker { m_implLock }; + if (impl_) + WebWorker__setRef(impl_, keepAlive); +} + +void Worker::clearZigImpl() +{ + Locker locker { m_implLock }; + impl_ = nullptr; } bool Worker::updatePtr() { if (!WebWorker__updatePtr(impl_, this)) { + clearZigImpl(); m_onlineClosingFlags = ClosingFlag; m_terminationFlags.fetch_or(TerminatedFlag); return false; @@ -263,7 +272,9 @@ void Worker::terminate() { // m_contextProxy.terminateWorkerGlobalScope(); m_terminationFlags.fetch_or(TerminateRequestedFlag); - WebWorker__notifyNeedTermination(impl_); + Locker locker { m_implLock }; + if (impl_) + WebWorker__notifyNeedTermination(impl_); } // const char* Worker::activeDOMObjectName() const @@ -468,6 +479,8 @@ void Worker::forEachWorker(const FunctiondispatchExit(exitCode); + // The Zig WebWorker is about to be freed; prevent terminate()/ref()/unref() from touching it. + worker->clearZigImpl(); // no longer referenced by Zig worker->deref(); diff --git a/src/bun.js/bindings/webcore/Worker.h b/src/bun.js/bindings/webcore/Worker.h index bbc73053ddfe..906d0d000bc4 100644 --- a/src/bun.js/bindings/webcore/Worker.h +++ b/src/bun.js/bindings/webcore/Worker.h @@ -76,6 +76,7 @@ class Worker final : public ThreadSafeRefCounted, public EventTargetWith void dispatchEvent(Event&); void dispatchCloseEvent(Event&); void setKeepAlive(bool); + void clearZigImpl(); void postTaskToWorkerGlobalScope(Function&&); @@ -119,6 +120,7 @@ class Worker final : public ThreadSafeRefCounted, public EventTargetWith // Tracks TerminateRequestedFlag and TerminatedFlag std::atomic m_terminationFlags { 0 }; const ScriptExecutionContextIdentifier m_clientIdentifier; + Lock m_implLock; void* impl_ { nullptr }; }; diff --git a/test/js/bun/test/mock-construct.test.ts b/test/js/bun/test/mock-construct.test.ts new file mode 100644 index 000000000000..9798eac46cde --- /dev/null +++ b/test/js/bun/test/mock-construct.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, mock, test } from "bun:test"; + +describe("mock() used as a constructor", () => { + test("Reflect.construct with no implementation returns an object", () => { + const m = mock(); + const r = Reflect.construct(m, []); + expect(typeof r).toBe("object"); + expect(r).not.toBeNull(); + }); + + test("Reflect.construct with a different newTarget returns an object", () => { + const m = mock(); + const r = Reflect.construct(m, [], function () {}); + expect(typeof r).toBe("object"); + expect(r).not.toBeNull(); + }); + + test("Reflect.construct with implementation returning a primitive returns an object", () => { + const m = mock(() => 42); + const r = Reflect.construct(m, []); + expect(typeof r).toBe("object"); + expect(r).not.toBeNull(); + }); + + test("Reflect.construct with mockReturnValue(primitive) returns an object", () => { + const m = mock().mockReturnValue(7); + const r = Reflect.construct(m, []); + expect(typeof r).toBe("object"); + expect(r).not.toBeNull(); + }); + + test("Reflect.construct preserves object return values", () => { + const obj = { hello: "world" }; + const m = mock(() => obj); + const r = Reflect.construct(m, []); + expect(r).toBe(obj); + }); + + test("calling without new still returns the implementation's value", () => { + const m1 = mock(); + expect(m1()).toBeUndefined(); + const m2 = mock(() => 42); + expect(m2()).toBe(42); + const m3 = mock().mockReturnValue("x"); + expect(m3()).toBe("x"); + }); +}); diff --git a/test/js/web/workers/worker-terminate-after-exit.test.ts b/test/js/web/workers/worker-terminate-after-exit.test.ts new file mode 100644 index 000000000000..42c6a5a7be7d --- /dev/null +++ b/test/js/web/workers/worker-terminate-after-exit.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, test } from "bun:test"; + +describe("Worker", () => { + test("terminate()/ref()/unref() after the worker has exited does not crash", async () => { + const url = URL.createObjectURL(new Blob(["/* exits immediately */"], { type: "application/javascript" })); + try { + const workers: Worker[] = []; + for (let i = 0; i < 4; i++) { + const w = new Worker(url); + workers.push(w); + } + + await Promise.all( + workers.map(w => { + const { promise, resolve } = Promise.withResolvers(); + w.addEventListener("close", () => resolve(), { once: true }); + return promise; + }), + ); + + Bun.gc(true); + + for (const w of workers) { + w.terminate(); + w.ref(); + w.unref(); + w.terminate(); + } + } finally { + URL.revokeObjectURL(url); + } + + expect(true).toBe(true); + }); +}); From 98a869b7b541d2e78398e691b73bd93cf8e03c81 Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 11 Apr 2026 22:49:19 +0000 Subject: [PATCH 2/3] Honor newTarget.prototype in mock construct fallback; register worker close listener before it can fire --- src/bun.js/bindings/JSMockFunction.cpp | 9 ++++++--- test/js/bun/test/mock-construct.test.ts | 7 +++++++ .../web/workers/worker-terminate-after-exit.test.ts | 12 +++++------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/bun.js/bindings/JSMockFunction.cpp b/src/bun.js/bindings/JSMockFunction.cpp index c1e8ff2d5dd2..01e9dc725b06 100644 --- a/src/bun.js/bindings/JSMockFunction.cpp +++ b/src/bun.js/bindings/JSMockFunction.cpp @@ -989,9 +989,12 @@ JSC_DEFINE_HOST_FUNCTION(jsMockFunctionConstruct, (JSGlobalObject * lexicalGloba JSC::EncodedJSValue encodedResult = jsMockFunctionCall(lexicalGlobalObject, callframe); RETURN_IF_EXCEPTION(scope, {}); JSValue result = JSValue::decode(encodedResult); - if (!result.isObject()) - return JSValue::encode(JSC::constructEmptyObject(lexicalGlobalObject)); - return encodedResult; + if (result.isObject()) + return encodedResult; + JSObject* newTarget = asObject(callframe->newTarget()); + Structure* structure = JSC::InternalFunction::createSubclassStructure(lexicalGlobalObject, newTarget, lexicalGlobalObject->objectStructureForObjectConstructor()); + RETURN_IF_EXCEPTION(scope, {}); + return JSValue::encode(JSC::constructEmptyObject(vm, structure)); } void JSMockFunctionPrototype::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject) diff --git a/test/js/bun/test/mock-construct.test.ts b/test/js/bun/test/mock-construct.test.ts index 9798eac46cde..3d55a95c20e3 100644 --- a/test/js/bun/test/mock-construct.test.ts +++ b/test/js/bun/test/mock-construct.test.ts @@ -36,6 +36,13 @@ describe("mock() used as a constructor", () => { expect(r).toBe(obj); }); + test("Reflect.construct with a custom newTarget honors its prototype", () => { + class Foo {} + const m = mock(); + const r = Reflect.construct(m, [], Foo); + expect(r).toBeInstanceOf(Foo); + }); + test("calling without new still returns the implementation's value", () => { const m1 = mock(); expect(m1()).toBeUndefined(); diff --git a/test/js/web/workers/worker-terminate-after-exit.test.ts b/test/js/web/workers/worker-terminate-after-exit.test.ts index 42c6a5a7be7d..76eb12071744 100644 --- a/test/js/web/workers/worker-terminate-after-exit.test.ts +++ b/test/js/web/workers/worker-terminate-after-exit.test.ts @@ -5,18 +5,16 @@ describe("Worker", () => { const url = URL.createObjectURL(new Blob(["/* exits immediately */"], { type: "application/javascript" })); try { const workers: Worker[] = []; + const exited: Promise[] = []; for (let i = 0; i < 4; i++) { + const { promise, resolve } = Promise.withResolvers(); const w = new Worker(url); + w.addEventListener("close", () => resolve(), { once: true }); workers.push(w); + exited.push(promise); } - await Promise.all( - workers.map(w => { - const { promise, resolve } = Promise.withResolvers(); - w.addEventListener("close", () => resolve(), { once: true }); - return promise; - }), - ); + await Promise.all(exited); Bun.gc(true); From 6c6de4fbe441d73399bcbd9ba5f3d8835b985572 Mon Sep 17 00:00:00 2001 From: robobun Date: Sat, 11 Apr 2026 23:36:42 +0000 Subject: [PATCH 3/3] Replace tautological assertion in worker-terminate-after-exit test --- test/js/web/workers/worker-terminate-after-exit.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/js/web/workers/worker-terminate-after-exit.test.ts b/test/js/web/workers/worker-terminate-after-exit.test.ts index 76eb12071744..4280e2b41933 100644 --- a/test/js/web/workers/worker-terminate-after-exit.test.ts +++ b/test/js/web/workers/worker-terminate-after-exit.test.ts @@ -24,10 +24,9 @@ describe("Worker", () => { w.unref(); w.terminate(); } + expect(workers.length).toBe(4); } finally { URL.revokeObjectURL(url); } - - expect(true).toBe(true); }); });