Closed Bug 2045428 Opened 5 months ago Closed 5 months ago

Assertion failure: this->is<T>(), at js/src/vm/JSObject.h:527 — type confusion in js::wasm::GetPromiseResults with a nuked cross-compartment promise (JSPI)

Categories

(Core :: JavaScript: WebAssembly, defect, P3)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
153 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox-esr140 --- unaffected
firefox151 --- disabled
firefox152 --- disabled
firefox153 --- fixed

People

(Reporter: decoder, Assigned: yury)

References

Details

(Keywords: ai-involved, sec-high, testcase)

Attachments

(1 file, 1 external file)

Summary

js::wasm::GetPromiseResults() in js/src/wasm/WasmPI.cpp (JS Promise Integration: WebAssembly.Suspending / WebAssembly.promising) performs an unchecked as<PromiseObject>() cast on an object that attacker-controlled JS can turn into a DeadObjectProxy while the wasm continuation is suspended. The existing dead-wrapper guard tests the wrong object shape, so a nuked cross-compartment wrapper bypasses it and a ProxyObject is then read as if it were a PromiseObject (NativeObject fixed-slot layout) — a memory-unsafe type confusion in release builds, an assertion failure in debug builds.

This is the direct sibling of the just-fixed Bug 2042295 (c1450bc54340 — "Detect dead cross-compartment promise in JSPI"), which hardened AddPromiseReactions in the same file but left GetPromiseResults with an insufficient check.

Root cause

js/src/wasm/WasmPI.cpp, GetPromiseResults (lines 1243–1257):

JSObject* promiseObj = &AnyRef::fromCompiledCode(promiseRef).toJSObject();
if (IsWrapper(promiseObj)) {
  promiseObj = UncheckedUnwrap(promiseObj);
  if (JS_IsDeadWrapper(promiseObj)) {
    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DEAD_OBJECT);
    return nullptr;
  }
}
Rooted<PromiseObject*> promise(cx, &promiseObj->as<PromiseObject>());  // <-- line 1257

The guard only catches a live wrapper whose unwrapped target is dead. When a CCW is nuked (js::NukeCrossCompartmentWrappers), the wrapper object itself is mutated in place: ProxyObject::nuke() (vm/ProxyObject.cpp:163) replaces its handler with DeadObjectProxy::singleton. After that:

  • IsWrapper(obj) is false (handler family no longer Wrapper::family, js/public/Wrapper.h:395), so the whole guard is skipped;
  • JS_IsDeadWrapper(obj) would be true, but is never consulted for this shape.

Execution falls through to promiseObj->as<PromiseObject>() on a ProxyObject; the subsequent PromiseObject accessors (state(), valueOrReason()) read NativeObject fixed slots out of a proxy layout, aliasing the proxy handler pointer / private slot as JS::Values.

How a CCW reaches the $promise local

The generated $suspending.exported function stores the result of $builtin.promise-resolve in the $promise local and reloads it after the suspend (WasmPI.cpp lines 159–190). PromiseResolve → CommonStaticResolveImpl (builtin/Promise.cpp:5968–5998) returns the input object unchanged when it canUnwrapAs<PromiseObject>() and x.constructor === Promise, so a wrapped promise from another compartment whose constructor was set to the importing realm's Promise flows verbatim into the wasm local.

While the continuation is suspended, arbitrary JS runs (a full event-loop turn). Nuking the wrapper in that window (in the browser this happens for real on document teardown / iframe navigation / worker shutdown; in the shell via the fuzzing-safe nukeCCW testing function) turns the object held by the suspended wasm frame into a DeadObjectProxy. The promise reaction that resumes the continuation was registered on the unwrapped target promise (JS::AddPromiseReactions → ReactToPromise unwraps, jsapi.cpp:3106), so it still fires after the nuke, resuming the continuation, which immediately calls GetPromiseResults with the dead proxy. The shell nukeCCW is only a convenience — CCW nuking is a core browser mechanism, and Bug 2042295 treated the equivalent scenario in AddPromiseReactions as a real bug.

Reproduction

Testcase: poc.js (attached).

js --fuzzing-safe poc.js

JSPI (javascript.options.wasm_js_promise_integration) is enabled by default on Nightly (value: @IS_NIGHTLY_BUILD@, StaticPrefList.yaml; off on release/beta), so no extra flags are needed — Nightly-only, but a supported configuration.

Steps performed by the PoC:

  1. Create promise q in a new compartment; obtain CCW p for it.
  2. p.constructor = Promise; so PromiseResolve(Promise, p) returns p itself.
  3. new WebAssembly.Suspending(() => p), import it into a small module, wrap the export with WebAssembly.promising, call it — the continuation suspends with the CCW in the $promise local.
  4. nukeCCW(p) — the wrapper becomes a DeadObjectProxy in place.
  5. Resolve q and drainJobQueue() — the reaction resumes the continuation; GetPromiseResults performs the bogus cast.

Crash signature

Debug ASan build, deterministic:

Assertion failure: this->is<T>(), at /builds/worker/checkouts/gecko/js/src/vm/JSObject.h:527
==722651==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 ...
    #0 MOZ_CrashSequence(void*, long) dist/include/mozilla/Assertions.h:261:3
    #1 js::PromiseObject& JSObject::as<js::PromiseObject>() js/src/vm/JSObject.h:527:5
    #2 js::wasm::GetPromiseResults(js::wasm::Instance*, void*, unsigned int) js/src/wasm/WasmPI.cpp:1257:51
    #3 ([anon:js-executable-memory]+0x2033c)

The opt ASan build runs the same type-confused reads (proxy internals reinterpreted as promise flags/result slots) but happens not to fault immediately; the debug assertion pinpoints the unchecked cast.

Suggested fix

In GetPromiseResults (and for symmetry in the builtin AddPromiseReactions), check for a dead proxy on the object itself before/in addition to the unwrap:

if (JS_IsDeadWrapper(promiseObj)) { /* report JSMSG_DEAD_OBJECT */ }
if (IsWrapper(promiseObj)) {
  promiseObj = UncheckedUnwrap(promiseObj);
  if (JS_IsDeadWrapper(promiseObj)) { /* report JSMSG_DEAD_OBJECT */ }
}

or replace the manual unwrap/cast with UnwrapAndTypeCheckValue<PromiseObject>.

Build

SpiderMonkey JS shell, mozilla-central asan-debug, product_version = 20260603-5cffec5992d3 (x86-64 Linux).

I'm pretty sure this is a duplicate of bug 2042295. Yury, can you confirm?

Severity: -- → S3
Flags: needinfo?(ydelendik)
Priority: -- → P3

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:rhunt, could you consider increasing the severity of this security bug?

For more information, please visit BugBot documentation.

Flags: needinfo?(rhunt)
Blocks: wasm-jspi
Severity: S3 → S2
Flags: needinfo?(rhunt)
Attached file (secure) —

I'm pretty sure this is a duplicate of bug 2042295.

Not a duplicate. It is using nukeCCW manipulation I don't fully understand yet. I suggested new change based on how we do things elsewhere in the code. jandem, can you provide feedback if nukeCCW-like test we have possible in the browser (for priority purpose)

Flags: needinfo?(ydelendik) → needinfo?(jdemooij)

(In reply to Yury Delendik (:yury) from comment #5)

jandem, can you provide feedback if nukeCCW-like test we have possible in the browser (for priority purpose)

Cross-compartment-wrapper nuking is used in the browser for cutting CCWs to content objects (to prevent leaks). Typical case is a CCW in chrome/add-on code that wraps an object in content code. When the website is navigated away from, the wrapper can be nuked.

It might be more of a sec-moderate because CCW nuking is not something content code can trigger on its own. Promises are complicated though and the add-on case makes it a bit more interesting, so I think we should fix this soon, but if there are other sec-highs I'd fix those first.

Flags: needinfo?(jdemooij)
Assignee: nobody → ydelendik
Status: NEW → ASSIGNED
Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 153 Branch

The patch landed in nightly and beta is affected.
:yury, is this bug important enough to require an uplift?

For more information, please visit BugBot documentation.

Flags: needinfo?(ydelendik)

The feature is disabled under the prefs in 152

Flags: needinfo?(ydelendik)
QA Whiteboard: [qa-triage-done-c154/b153]
QA Whiteboard: [qa-triage-done-c154/b153] → [qa-triage-done-c154/b153][sec]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: