2027991 - Type confusion in [@ js::Proxy::getInternal] due to incorrect AliasSet::None on MGuardIsProxy allowing GVN elimination across transplant

Status: RESOLVED DUPLICATE of bug 2027984

(Core :: JavaScript Engine, defect)

Description

[Bugmon [:jkratzer for issues]]

Summary

The GuardIsProxy MIR instruction is declared with alias_set: none in js/src/jit/MIROps.yaml (line 2633). Combined with congruent_to: if_operands_equal and movable: true, this permits Global Value Numbering to merge two MGuardIsProxy instructions that guard the same SSA operand even when an arbitrary effectful call sits between them. If that call transplants the guarded object (swapping its guts from a ProxyObject to a NativeObject), the second, now-eliminated guard would have bailed out — instead Ion proceeds directly to the VM call ProxyGetProperty on a non-proxy object. Inside Proxy::getInternal, the unchecked proxy->as<ProxyObject>().handler() reads the ProxyObject::data.handler slot, which overlaps with NativeObject::elements_. The resulting pointer (js::emptyObjectElements) is then dereferenced as a BaseProxyHandler*, producing an out-of-bounds read in hasSecurityPolicy() and subsequently a virtual call through an attacker-influenceable vtable.

Affected Code

File: js/src/jit/MIROps.yaml, line 2626-2636

- name: GuardIsProxy
  operands:
    object: Object
  result_type: Object
  guard: true
  movable: true
  congruent_to: if_operands_equal
  alias_set: none          # <-- BUG: should depend on ObjectFields
  generate_lir: true
  lir_result_type: none
  lir_temps: 1

alias_set: none tells the alias analysis that this instruction reads no mutable memory. But the guard actually reads the object's class (via the shape/group) to determine proxy-ness. Object transplant (JSObject::swap) mutates exactly this field. Because GVN only refuses to merge congruent instructions when an intervening store aliases the instruction's load set, an empty load set means the two guards are always mergeable regardless of intervening side effects.

The consumer of this guard, tryAttachGenericProxy in CacheIR.cpp, emits:

writer.guardIsProxy(objId);
...
writer.proxyGetResult(objId, id);  // transpiles to MProxyGet → ProxyGetProperty VM call

After Warp transpilation and GVN, the second proxyGetResult runs without its protecting guard.

Correct Pattern

- name: GuardIsProxy
  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

This causes GVN to treat any store to ObjectFields (including the AliasSet::Store(Any) emitted for arbitrary calls) as invalidating the guard.

Exploit Chain

1. Create a transplantable DOM-like object and move it into a foreign-zone compartment so it becomes a cross-compartment wrapper (a ProxyObject).
2. Warm up test(obj) { obj.foo; cb(); obj.foo; } so the Baseline IC attaches the GenericProxy stub (guardIsProxy + proxyGetResult) for both property reads. The foreign-zone CCW skips the zone-specific CCW stub, ensuring the transpileable generic stub is chosen.
3. Ion compiles test. WarpCacheIRTranspiler emits two MGuardIsProxy(obj) + MProxyGet(obj) pairs separated by the call to cb().
4. GVN observes both MGuardIsProxy nodes have identical operands and AliasSet::None; the second is replaced by the first. The second MProxyGet now consumes the pre-call guard result.
5. On the trigger iteration, cb() calls transplant(globalThis), swapping domObj back into the current zone. The object header is rewritten to a NativeObject subclass. No Ion invalidation fires because the IC shape guards were on the wrapper, not the script.
6. Execution returns to Ion and falls straight into the second MProxyGet → ProxyGetProperty(cx, obj, ...).
7. Proxy::getInternal executes proxy->as<ProxyObject>().handler(). The ProxyObject::data.handler field lives at the same offset as NativeObject::elements_, so the read returns the static emptyObjectElements pointer.
8. AutoEnterPolicy calls handler->hasSecurityPolicy(), reading mHasSecurityPolicy at offset 0x11 into the bogus handler. This lands 9 bytes past the 24-byte emptyElementsHeader global → ASAN global-buffer-overflow. In a non-ASAN build, execution continues to handler->enter(...) / handler->get(...), performing a virtual call through a vtable pointer read from ObjectElements data.

Security Impact

Severity: High (sec-high). Type confusion between ProxyObject and NativeObject with a subsequent virtual call through confused memory. An attacker who can place controlled data in a NativeObject's elements header (or arrange for a non-empty elements buffer) controls the fake BaseProxyHandler vtable pointer, leading to arbitrary code execution in the content process.

Preconditions: Requires Ion compilation and the ability to trigger an object transplant mid-function. In the browser, transplants occur during document.adoptNode, cross-global navigation, and WindowProxy re-targeting — all reachable from web content. The shell testcase uses transplantableObject() as a stand-in for these paths.

Suggested Fix

Change the alias set of GuardIsProxy (and audit GuardIsNativeObject, GuardIsNotProxy which share the same alias_set: none pattern) to depend on ObjectFields:

 - name: GuardIsProxy
   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

This ensures any intervening instruction that stores to ObjectFields (all calls emit AliasSet::Store(AliasSet::Any)) blocks GVN from eliminating the redundant guard.

Comment 1

[Bugmon [:jkratzer for issues]]

Attachment: poc.js

Comment 2

[Bugmon [:jkratzer for issues]]

Attachment: crash_stack.txt

Comment 5

[Bugmon [:jkratzer for issues]]

No valid actions for resolution (DUPLICATE).
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.