2027989 - Type confusion in [@ js::jit::HasNativeElementPure] due to incorrect alias_set on MGuardIsNativeObject allowing GVN across JSObject::swap

Status: RESOLVED DUPLICATE of bug 2027984

(Core :: JavaScript Engine: JIT, defect)

Description

[Bugmon [:jkratzer for issues]]

Summary

MGuardIsNativeObject is declared with alias_set: none in js/src/jit/MIROps.yaml (line 2608). Combined with movable: true and congruent_to: if_operands_equal, GVN treats the guard as a pure operation and eliminates redundant instances even when an intervening call mutates the object's class via JSObject::swap (transplant). After the swap converts the object into a CrossCompartmentWrapper proxy, the eliminated second guard allows MCallObjectHasSparseElement to invoke HasNativeElementPure with a ProxyObject cast as NativeObject*. The proxy's handler field (which occupies the same offset as NativeObject::elements_) is dereferenced with a negative ObjectElements header offset, producing a global-buffer-overflow read 12 bytes before js::CrossCompartmentWrapper::singleton.

Affected Code

File: js/src/jit/MIROps.yaml, line 2601–2611

# 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: should be load: ObjectFields
  generate_lir: true
  lir_result_type: none
  lir_temps: 1

With alias_set: none, MGuardIsNativeObject is assumed to depend on no mutable memory. GVN (ValueNumberer::visitDefinition) sees two guards on the same SSA operand as congruent and replaces the second with the first, even though a non-inlined call sits between them. A call's store alias set includes ObjectFields, which would normally block this deduplication — but only if the guard actually declares a load dependency on ObjectFields.

An object's class (native vs. proxy) is stored in its shape/base-shape, which is part of the ObjectFields alias category. JSObject::swap (invoked by DOM transplants and the shell transplantableObject helper) rewrites this field in place, so any guard that inspects it must alias ObjectFields.

Crash site: js/src/vm/NativeObject.h:729–735

bool containsDenseElement(uint32_t idx) const {
  return idx < getDenseInitializedLength() &&
         !elements_[idx].isMagic(JS_ELEMENTS_HOLE);
}
uint32_t getDenseInitializedLength() const {
  return getElementsHeader()->initializedLength;   // reads *(elements_ - sizeof(ObjectElements) + 4)
}

On a ProxyObject, offset offsetof(NativeObject, elements_) holds the proxy handler pointer (&CrossCompartmentWrapper::singleton). getElementsHeader() subtracts sizeof(ObjectElements) and reads initializedLength, landing 12 bytes before the singleton global.

Correct Pattern

- 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

The same fix is required for the sibling ops that read the object's class/shape to discriminate proxy-ness: GuardIsProxy, GuardIsNotProxy, GuardIsNotDOMProxy, GuardHasProxyHandler, LoadScriptedProxyHandler, and IsConstructor if they were similarly modified.

Exploit Chain

1. Script obtains a transplantable NativeObject and assigns a sparse index (obj[1000000] = 42) so that HasPropIRGenerator::tryAttachSparse fires.
2. Object.hasOwn(obj, 1000000) attaches a CacheIR stub that emits guardIsNativeObject(objId) followed by callObjectHasSparseElementResult(objId, indexId) (CacheIR.cpp:3905–3916).
3. Warp compiles f(o, idx). WarpCacheIRTranspiler::emitGuardIsNativeObject emits two MGuardIsNativeObject(obj) instructions — one before and one after the non-inlined callIt() (blocked from Ion by with ({}) {}).
4. GVN runs. Both guards have the same operand, congruent_to: if_operands_equal, and alias_set: none, so the second guard is replaced by the first. The second MCallObjectHasSparseElement now consumes the pre-call guard's result directly.
5. At runtime the callback is swapped to invoke transplant(g), which calls JSObject::swap and rewrites obj in place as a CrossCompartmentWrapper proxy.
6. Execution returns to Ion code. The (eliminated) second guard never re-checks the class. LCallObjectHasSparseElement calls HasNativeElementPure(cx, obj, idx, vp) with a ProxyObject* reinterpreted as NativeObject*.
7. obj->containsDenseElement(index) → getDenseInitializedLength() → getElementsHeader()->initializedLength dereferences (&CrossCompartmentWrapper::singleton) - sizeof(ObjectElements) + offsetof(ObjectElements, initializedLength), a 4-byte OOB read 12 bytes before the global.

Security Impact

Severity: sec-high. This is a JIT type confusion between NativeObject and ProxyObject. The immediate manifestation is an OOB read of a global redzone, but the confused object flows into further NativeObject accessors (elements_[idx], shape()->lookup, as<TypedArrayObject>()), giving an attacker who controls the swapped-in proxy layout a primitive for arbitrary-address reads and likely memory corruption. In the browser, JSObject::swap is reachable via DOM node adoption / document.adoptNode and WindowProxy transplants, so no shell-only primitive is strictly required.

Preconditions: Warp/Ion compilation of a function containing two Object.hasOwn (or in) operations on a sparse-indexed native object with an intervening call, plus the ability to trigger a transplant of that object during the call.

Reproduction

Run with: js --fuzzing-safe --fast-warmup --no-threads poc.js

Suggested Fix

Restore the load alias set on GuardIsNativeObject (and the related proxy-discriminating guards) so GVN cannot hoist/dedupe them across calls that may invoke JSObject::swap:

 - 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

Apply the same change to GuardIsProxy, GuardIsNotProxy, GuardIsNotDOMProxy, GuardHasProxyHandler, LoadScriptedProxyHandler, and IsConstructor.

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.