Low firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionJIT miscompilation in the JavaScript Engine: JIT component
ComponentSpiderMonkey
Bug ClassLogic Error
Tracker1952164
Fix commit4abff7d74bb3 (firefox) +44/-25
CISA KEVNot listed
Creditedanbu
Disclosed2026-08-18

Background

`MInt32ToIntPtr`
a SpiderMonkey MIR node that widens a 32-bit Int32 value into a pointer-width IntPtr, used mainly for typed-array/DataView indices.
No-op redefine lowering
a lowering that emits no code and uses redefine so the result shares the input’s virtual register and 32-bit spill slot instead of allocating a real LInt32ToIntPtr conversion.
Bounds-check elimination
a JIT optimization that removes or hoists array bounds checks, after which an index is treated as known non-negative.
Spill slot
a stack location where the register allocator stores a value that cannot stay in a register; a 32-bit value occupies only a 4-byte slot.

Root Cause Analysis

LIRGenerator::visitInt32ToIntPtr chose a no-op redefine lowering whenever the value was proven non-negative, letting the IntPtr result reuse the input Int32’s virtual register and 4-byte spill slot. This is only sound when every use reads the value from a register, where an Int32 is zero-extended and coincides with the sign-extended non-negative IntPtr; a use that instead reads the value at pointer width from the 4-byte spill slot would additionally consume bytes from the adjacent stack slot, yielding a corrupt IntPtr. The old code tracked only canBeNegative and implicitly assumed all non-negative-index consumers were register reads, violating the invariant that the shared narrow spill slot is never read at pointer width.

The fix scans every use and additionally computes canRedefine, only permitting the no-op lowering for the specific bounds-checked consumers (MLoadUnboxedScalar, MStoreUnboxedScalar, MLoadDataViewElement, MStoreDataViewElement, MSpectreMaskIndex, MBoundsCheck) that read the index from a register. For any other use it sets canRedefine = false and emits a real LInt32ToIntPtr conversion via useAnyAtStart, so no consumer ever reads a widened value out of a 32-bit slot.

Key insight
The single mistake was equating “index is non-negative” with “the no-op redefine is safe,” when safety actually requires that no consumer reads the widened value from the shared 32-bit spill slot; the fix separates the canRedefine decision from canBeNegative and falls back to a real conversion whenever an unrecognized use could read the value at pointer width.

Attack Path

  1. Shape the graph Craft JavaScript whose typed-array indexing produces an MInt32ToIntPtr whose result flows into a use other than the whitelisted bounds-checked load/store operands.
  2. Trigger bounds-check elimination Arrange the code so the JIT proves the index non-negative and removes the bounds check, marking the node with setCanNotBeNegative.
  3. Force a spill Apply enough register pressure that the shared value is spilled to its 4-byte stack slot rather than kept in a register.
  4. Provoke the widened read Cause the non-whitelisted consumer to read the value at pointer width, pulling in the adjacent stack slot’s bytes and producing a wrong IntPtr.
  5. Observe miscompilation The corrupted index or derived value causes the JIT’d code to compute an incorrect result versus the interpreter.

Impact Assessment

An attacker who runs script in the content process can cause the optimizing JIT to miscompile pointer-width index handling, yielding an incorrect IntPtr and program result. Realizing it requires a rare register-spill layout plus an unusual consumer of the MInt32ToIntPtr result, and the reported impact is low (a logic/miscompilation error); the diff does not establish a controlled out-of-bounds or memory-corruption primitive.

Changed Functions

FunctionChangeNotes
if
js/src/jit/Lowering.cpp
modified

Files Changed

  • js/src/jit/CodeGenerator.cpp
  • js/src/jit/Lowering.cpp
  • js/src/jit/MIR.h

Audit Directions

  • redefine/no-op lowerings
    Audit every LIR lowering that shares a virtual register or spill slot via redefine for widening/narrowing mismatches, confirming all consumers read the value at the width the shared slot actually holds.
  • Non-negativity assumptions
    Review other JIT sites that infer setCanNotBeNegative from downstream uses and verify the use enumeration is exhaustive rather than relying on an implicit whitelist.
  • Operand-index asserts
    When adding MOZ_RELEASE_ASSERT on operand positions, confirm the new Name##Operand constants match the lowering’s actual useRegister* choices for each consumer type.
diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp
index 1926b46c308..efcdc0c8d2c 100644
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -1178,9 +1178,6 @@ void CodeGenerator::visitFloat32ToInt32(LFloat32ToInt32* lir) {
 
 void CodeGenerator::visitInt32ToIntPtr(LInt32ToIntPtr* lir) {
 #ifdef JS_64BIT
-  // This LIR instruction is only used if the input can be negative.
-  MOZ_ASSERT(lir->mir()->canBeNegative());
-
   Register output = ToRegister(lir->output());
   const LAllocation* input = lir->input();
   if (input->isGeneralReg()) {
diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp
index bcbcde85983..73843102489 100644
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -3631,30 +3631,50 @@ void LIRGenerator::visitInt32ToIntPtr(MInt32ToIntPtr* ins) {
   // If the result is only used by instructions that expect a bounds-checked
   // index, we must have eliminated or hoisted a bounds check and we can assume
   // the index is non-negative. This lets us generate more efficient code.
-  if (ins->canBeNegative()) {
-    bool canBeNegative = false;
-    for (MUseDefIterator iter(ins); iter; iter++) {
-      if (iter.def()->isSpectreMaskIndex()) {
-        continue;
-      }
-      if (iter.def()->isLoadUnboxedScalar() ||
-          iter.def()->isStoreUnboxedScalar() ||
-          iter.def()->isLoadDataViewElement() ||
-          iter.def()->isStoreDataViewElement()) {
-        MOZ_ASSERT(iter.def()->indexOf(iter.use()) == 1,
-                   "unexpected non-index operand use");
-        continue;
-      }
-
-      canBeNegative = true;
-      break;
+  //
+  // The no-op lowering with |redefine| shares the input's virtual register and
+  // 32-bit spill slot, so all instruction uses must read from a register:
+  // registers hold Int32 values zero-extended, which matches the sign-extended
+  // IntPtr value because we only do this when the value is non-negative. A
+  // pointer-width memory read would instead also consume bytes from the
+  // adjacent stack slot. Snapshots handle this separately with
+  // RValueAllocation::IntPtrInt32.
+  bool canBeNegative = false;
+  bool canRedefine = true;
+  for (MUseDefIterator iter(ins); iter; iter++) {
+    MDefinition* def = iter.def();
+    size_t index = def->indexOf(iter.use());
+    if (def->isLoadUnboxedScalar() || def->isStoreUnboxedScalar() ||
+        def->isLoadDataViewElement() || def->isStoreDataViewElement()) {
+      // The bounds-checked index is lowered with useRegisterOrIndexConstant.
+      static_assert(MLoadUnboxedScalar::indexOperand == 1);
+      static_assert(MStoreUnboxedScalar::indexOperand == 1);
+      static_assert(MLoadDataViewElement::indexOperand == 1);
+      static_assert(MStoreDataViewElement::indexOperand == 1);
+      MOZ_RELEASE_ASSERT(index == 1);
+      continue;
+    }
+    if (def->isSpectreMaskIndex() && index == MSpectreMaskIndex::indexOperand) {
+      // The bounds-checked index is lowered with useRegister.
+      continue;
     }
-    if (!canBeNegative) {
-      ins->setCanNotBeNegative();
+    if (def->isBoundsCheck() && index == MBoundsCheck::indexOperand) {
+      // The index operand is lowered with useRegisterOrInt32Constant, but
+      // because this is the instruction performing the check it implies
+      // nothing about the sign.
+      canBeNegative = true;
+      continue;
     }
+    // Don't optimize other uses.
+    canBeNegative = true;
+    canRedefine = false;
+    break;
+  }
+  if (!canBeNegative) {
+    ins->setCanNotBeNegative();
   }
 
-  if (ins->canBeNegative()) {
+  if (ins->canBeNegative() || !canRedefine) {
     auto* lir = new (alloc()) LInt32ToIntPtr(useAnyAtStart(input));
     define(lir, ins);
   } else {
diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h
index cef95b493d6..cb77524214a 100644
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -1137,8 +1137,10 @@ class MInstruction : public MDefinition, public InlineListNode<MInstruction> {
 //     NAMED_OPERANDS((0, lhs), (1, rhs))
 //
 // The above example defines 2 accessors, one named "lhs" accessing the first
-// operand, and a one named "rhs" accessing the second operand.
-#define NAMED_OPERAND_ACCESSOR(Index, Name) \
+// operand, and a one named "rhs" accessing the second operand. It also defines
+// the operand indices as |lhsOperand| and |rhsOperand|.
+#define NAMED_OPERAND_ACCESSOR(Index, Name)      \
+  static constexpr size_t Name##Operand = Index; \
   MDefinition* Name() const { return getOperand(Index); }
 #define NAMED_OPERAND_ACCESSOR_APPLY(Args) NAMED_OPERAND_ACCESSOR Args
 #define NAMED_OPERANDS(...) \
Loading diff…