2024919 - Type confusion in Warp-compiled [@ js::NativeGetElement] due to incorrect `alias_set: none` on MGuardIsNativeObject

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Bugmon [:jkratzer for issues]]

Summary

Vulnerability type: Type confusion (JIT optimizer alias-set misconfiguration)

Affected file: js/src/jit/MIROps.yaml, line 2607

Root cause: MGuardIsNativeObject declares alias_set: none, telling alias analysis that the instruction reads no memory. However, its codegen (MacroAssembler::branchIfNonNativeObj) loads obj->shape() and tests shape->immutableFlags & isNativeBit. The shape pointer is a mutable object field that can be rewritten by JSObject::swap during cross-compartment transplant. Because AliasAnalysis::analyze skips None alias sets (if (set.isNone()) continue;), both instances of the guard receive dependency() == nullptr. GVN's congruence check (ValueNumberer::VisibleValues::ValueHasher::match) then merges them across an effectful call that swaps the object into a proxy, and the stale guard output flows into MCallNativeGetElement, which passes a ProxyObject* as Handle<NativeObject*> to js::NativeGetElement.

Affected Code

File: js/src/jit/MIROps.yaml, lines 2599–2610

# Guard the object is a native object.
- name: GuardIsNativeObject
  operands:
    object: Object
  result_type: Object
  guard: true
  movable: true
  congruent_to: if_operands_equal
  alias_set: none            # BUG: codegen reads obj->shape() (ObjectFields)
  generate_lir: true
  lir_result_type: none
  lir_temps: 1

Why it is vulnerable:

The generated code (CodeGenerator::visitGuardIsNativeObject → MacroAssembler::branchIfNonNativeObj) performs:

void MacroAssembler::branchIfNonNativeObj(Register obj, Register scratch,
                                          Label* label) {
  loadPtr(Address(obj, JSObject::offsetOfShape()), scratch);   // ← load ObjectFields
  branchTest32(Assembler::Zero,
               Address(scratch, Shape::offsetOfImmutableFlags()),
               Imm32(Shape::isNativeBit()), label);
}

The shape_ word is overwritten by JSObject::swap (JSObject.cpp:1289–1291):

js_memcpy(tmp, a, size);
js_memcpy(a, b, size);   // overwrites a->shape_ with b->shape_
js_memcpy(b, tmp, size);

ObjectMayBeSwapped explicitly permits DOM-class objects (clasp->isDOMClass()) and proxies to be swapped, so a native DOM object can become a CrossCompartmentWrapper proxy at the same heap address — the exact transition this guard exists to catch.

Compare with the immediately preceding entries GuardShapeListToOffset (line 2577) and GuardMultipleShapesToOffset (line 2590), which also load obj->shape() and correctly declare alias_set: load: ObjectFields.

Correct Pattern

# Guard the object is a native object.
- name: GuardIsNativeObject
  operands:
    object: Object
  result_type: Object
  guard: true
  movable: true
  congruent_to: if_operands_equal
  alias_set:
    load: ObjectFields
  generate_lir: true
  lir_result_type: none
  lir_temps: 1

With load: ObjectFields, alias analysis assigns a store dependency pointing to the preceding effectful MCallNativeGetElement / MCallGetProperty (the trigger[0] getter call). The two guard instances then have different dependency() values and fail the GVN congruence check at ValueNumbering.cpp:53, so the second guard stays in place and bails out after the swap.

Exploit Chain

1. Attach megamorphic IC. Create 10 TransplantableDOMObject instances, each with a unique named property (p0..p9, producing distinct shapes) and a sparse element [1000000]. Executing o[1000000] in a loop over all 10 drives the GetElem IC megamorphic. GetPropIRGenerator::tryAttachGenericElement attaches the GenericElementMegamorphic stub: guardIsNativeObject(objId) → guardIndexIsNotDenseElement(objId, indexId) → callNativeGetElementResult(objId, indexId).

2. Warp transpilation. After warmup, Warp transpiles the stub via WarpCacheIRTranspiler::emitGuardIsNativeObject and emitCallNativeGetElementResult. Function f contains two o[1000000] accesses separated by trigger[0] (a getter with an effectful call), producing MIR roughly:

   g1  = MGuardIsNativeObject(o)        ; alias_set=None,  dependency=nullptr
   ...   MGuardIndexIsNotDenseElement(g1, 1000000)
   c1  = MCallNativeGetElement(g1, 1000000)   ; effectful, stores Any
   ...   MCallGetProperty(trigger, 0)         ; effectful — runs JS getter
   g2  = MGuardIsNativeObject(o)        ; alias_set=None,  dependency=nullptr
   ...   MGuardIndexIsNotDenseElement(g2, 1000000)
   c2  = MCallNativeGetElement(g2, 1000000)
   

3. Alias analysis skip. AliasAnalysis::analyze encounters both guards with set.isNone() and executes continue;, leaving dependency_ = nullptr on both. (In contrast, MGuardIndexIsNotDenseElement reads ObjectFields and correctly picks up a dependency on the preceding effectful call — but its object operand comes from the guard, not directly from o.)

4. GVN merge. ValueNumberer::VisibleValues::ValueHasher::match compares g1 and g2: same opcode, same operand (o), same dependency() (nullptr). congruentTo (generated from congruent_to: if_operands_equal) returns true. g2 is replaced by g1. The second MGuardIndexIsNotDenseElement and MCallNativeGetElement now both consume g1's output — which is just o passed through, checked once before any JS ran.

5. Transplant. On the final iteration, doTransplant = true causes the trigger[0] getter to call targetTransplant(g2), which invokes JS_TransplantObject → JSObject::swap(target, newIdentityWrapper). The heap memory at target's address now holds a CrossCompartmentWrapper proxy: target->shape()->isNative() == false, and the word at offsetof(NativeObject, elements_) (= offset 16) now holds the proxy's handler pointer &js::CrossCompartmentWrapper::singleton.

6. Stale guard output. Execution reaches the second element access. No guard runs (it was merged away). CodeGenerator::visitCallNativeGetElement pushes the object register (still holding target's address) and calls js::NativeGetElement(cx, Handle<NativeObject*>{proxy_ptr}, ...).

7. Type-confused dereference. NativeGetElement → NativeGetProperty → NativeGetPropertyInline → NativeLookupOwnPropertyInline calls obj->containsDenseElement(1000000) → getDenseInitializedLength() → getElementsHeader()->initializedLength. getElementsHeader() computes reinterpret_cast<ObjectElements*>(elements_ - sizeof(ObjectElements)). With elements_ holding &CrossCompartmentWrapper::singleton and offsetof(ObjectElements, initializedLength) - sizeof(ObjectElements) == -12 (NativeObject.h:376-379), the 4-byte read lands at &CrossCompartmentWrapper::singleton - 12 — inside the ASAN global redzone between the CrossCompartmentWrapper vtable and its singleton instance.

Steps to Reproduce

No environment changes required. Run the testcase in an ASAN JS shell with:

js --fuzzing-safe --no-threads --stub-folding=off --baseline-warmup-threshold=10 --ion-warmup-threshold=100 poc.js

--stub-folding=off keeps the megamorphic stub in a form Warp transpiles directly; --no-threads ensures deterministic Ion compilation ordering.

Security Impact

Severity: High (sec-high). This is a JIT-level type confusion between ProxyObject and NativeObject reaching C++ VM code.

Attacker capability:

• A ProxyObject* is passed as Handle<NativeObject*> to NativeGetElement, which treats proxy internals (handler pointer, reserved slots) as NativeObject::elements_.
• In a non-ASAN build, getDenseInitializedLength() reads 4 bytes at handler_ptr - 12. If that value happens to exceed the requested index (1000000), the next step elements_[idx].isMagic(JS_ELEMENTS_HOLE) reads 8 bytes at handler_ptr + idx*8 — an attacker-influenced large out-of-bounds read into .rodata/.data, potentially leakable through the returned property value.
• More broadly, any NativeObject method reachable from NativeGetPropertyInline on this path operates on proxy memory with the wrong layout interpretation. Proto-chain walking, dense element reads, and slot accesses all become wild reads.

Preconditions:

• Requires Ion/Warp compilation of a function with two GuardIsNativeObject-backed element accesses to the same object, separated by a call that can trigger JS_TransplantObject.
• In the browser, transplant is triggered by cross-compartment navigation primitives (e.g. document.adoptNode, document.domain changes, WindowProxy remapping). The shell testcase uses transplantableObject() as a stand-in for a DOM reflector.
• The target object must satisfy ObjectMayBeSwapped (isDOMClass() or isProxyObject()) — all DOM reflectors qualify.

Related latent bugs: GuardIsProxy (MIROps.yaml:2632), GuardIsNotDOMProxy (:2644), and GuardIsNotProxy (:2657) have the same alias_set: none declaration and also read obj->shape() in their codegen. They are likely vulnerable to the same GVN merge across a swap.

ASAN Report

==636193==ERROR: AddressSanitizer: global-buffer-overflow on address 0x57b8289441f4 at pc 0x57b826f8b44e bp 0x7fff437d7750 sp 0x7fff437d7748
READ of size 4 at 0x57b8289441f4 thread T0
    #0 0x57b826f8b44d in js::NativeObject::getDenseInitializedLength() const /firefox/js/src/vm/NativeObject.h:734:33
    #1 0x57b826f8b44d in js::NativeObject::containsDenseElement(unsigned int) const /firefox/js/src/vm/NativeObject.h:730:18
    #2 0x57b826f8b44d in bool js::NativeLookupOwnPropertyInline<(js::AllowGC)1, (js::LookupResolveMode)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, js::PropertyResult*) /firefox/js/src/vm/NativeObject-inl.h:730:14
    #3 0x57b826f8b44d in bool NativeGetPropertyInline<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, IsNameLookup, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) /firefox/js/src/vm/NativeObject.cpp:2306:10
    #4 0x57b826f8b44d in js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyKey>, JS::MutableHandle<JS::Value>) /firefox/js/src/vm/NativeObject.cpp:2353:10
    #5 0x57b826f8c854 in js::NativeGetElement(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, int, JS::MutableHandle<JS::Value>) /firefox/js/src/vm/NativeObject.cpp:2379:10
    #6 0x3bea6a01fab2  (<unknown module>)

0x57b8289441f4 is located 12 bytes before global variable 'js::CrossCompartmentWrapper::singleton' defined in '/firefox/obj-x86_64-pc-linux-gnu/dist/include/js/Wrapper.h:262' (0x57b828944200) of size 24
0x57b8289441f4 is located 52 bytes after global variable 'vtable for js::CrossCompartmentWrapper' defined in 'Unified_cpp_js_src_proxy0.cpp' (0x57b828944080) of size 320
SUMMARY: AddressSanitizer: global-buffer-overflow /firefox/js/src/vm/NativeObject.h:734:33 in js::NativeObject::getDenseInitializedLength() const

Suggested Fix

Change the alias set to reflect the actual memory read. In js/src/jit/MIROps.yaml:

 # Guard the object is a native object.
 - name: GuardIsNativeObject
   operands:
     object: Object
   result_type: Object
   guard: true
   movable: true
   congruent_to: if_operands_equal
-  alias_set: none
+  alias_set:
+    load: ObjectFields
   generate_lir: true
   lir_result_type: none
   lir_temps: 1

The same fix should be applied to GuardIsProxy, GuardIsNotDOMProxy, and GuardIsNotProxy, whose codegen paths (MacroAssembler::branchTestObjectIsProxy and related) also load obj->shape().

Comment 1

[Bugmon [:jkratzer for issues]]

Attachment: Crash stack trace

Comment 2

[Bugmon [:jkratzer for issues]]

Attachment: Testcase: tmpmj2t6oho.js

Comment 4

[Jan de Mooij [:jandem]]

Great find. This is a problem with 'brain transplants', a mechanism that swaps the guts of certain JS objects (eg a DOM object with a cross-compartment wrapper). This is the only way for a native object in SpiderMonkey to become non-native (a proxy).

I'll post patches to fix up the alias sets of various MIR instructions to account for this. Longer term I want to think about what it would take to get rid of brain transplants and JSObject::swap, at least for the case involving native (non-proxy) objects.

I expect this patch to be fine performance-wise. If it does cause performance issues we could add a fuse to keep the current behavior and deoptimize the first time an object is swapped.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: (secure)

Comment 6

[Jan de Mooij [:jandem]]

Attachment: (secure)

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 9557345 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Not trivial but doable.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: All
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: Not hard but this needs different patches for ESR115 and ESR140
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely. It makes some JIT optimizations more conservative.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 9557345 [details]
(secure)

sec-approval+ to land and request uplifts; hold off on landing the test until May (I'll set a reminder)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f42fd034bc8f
https://hg.mozilla.org/integration/autoland/rev/a06bf73a1f1e
Improve alias sets of some MIR instructions. r=iain

Comment 10

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: Security bugs.
• Code covered by automated testing?: yes
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Makes some JIT optimizations a bit more conservative; should be low risk.
• String changes made/needed?: N/A
• Is Android affected?: yes

Comment 11

[Jan de Mooij [:jandem]]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D289518

Comment 12

[Jan de Mooij [:jandem]]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D289518

Comment 13

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined/Reason for urgency: Security bugs.
• Code covered by automated testing?: yes
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Makes some JIT optimizations a bit more conservative; should be low risk.
• String changes made/needed?: N/A
• Is Android affected?: yes

Comment 14

[Phabricator Automation]

firefox-esr115 Uplift Approval Request

• User impact if declined/Reason for urgency: Security bugs.
• Code covered by automated testing?: yes
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Makes some JIT optimizations a bit more conservative; should be low risk.
• String changes made/needed?: N/A
• Is Android affected?: yes

Comment 15

[Jan de Mooij [:jandem]]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D289518

Comment 16

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/mozilla-central/rev/a06bf73a1f1e

Comment 17

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/7d0e94bab49b
https://hg.mozilla.org/releases/mozilla-beta/rev/4d89cf31ed87

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/365191506976

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/b8b1304e3bd5

Comment 22

[BugBot [:suhaib / :marco]]

2 months ago, dveditz placed a reminder on the bug using the whiteboard tag [reminder-test 2026-05-19] .

jandem, please refer to the original comment to better understand the reason for the reminder.

Comment 23

[Jan de Mooij [:jandem]]

For posterity, we've removed support for this class of bugs (swapping a native object with another object). We can only swap two proxy objects now and that's a lot less crazy and error-prone.

See bug 2037186. This was made possible by DOM changes in bug 2037172 and bug 1470017.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3c9f6f762b57
https://hg.mozilla.org/integration/autoland/rev/5868d979e9d2
Add comments. r=iain

Comment 25

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5868d979e9d2