[3] Array rematerialization should know how to have a bad time

HighJSC FTL JITTypeConfusion

The array that came back from an OSR exit disagreed with itself about its layout.

eba64ef

High. The exit-time reconstruction path asks the global object for a structure while hand-building a butterfly whose layout was decided at compile time — and script controls both halves of that mismatch. The result is the classic JSC structure/butterfly confusion, with the pointer-typed m_sparseMap slot overlaying attacker-written doubles.

WebKit's top-tier JavaScript optimizer can delete an object allocation entirely when the object never escapes the compiled region, recording only a descriptor of what the object would have contained. If the optimized code later bails out to a lower tier, that descriptor has to be turned back into a real heap object at exit time. Two pieces meet in that path: a JSArray's structure, the type descriptor recording (among other things) how its elements are laid out, and its butterfly, the heap block that actually holds them. The invariant is that the structure installed on a reconstructed array must describe the butterfly the reconstruction code actually built.

The angle: a page can make the engine produce a JSArray whose type says SlowPutArrayStorage while its backing store is contiguous, so every later indexed access reads and writes at a fixed displacement and reinterprets attacker-chosen doubles as array metadata including a pointer field.

The commit message states the shape exactly: sunk arrays rematerialized with a butterfly are always rematerialized with a contiguous butterfly, so if the VM had a bad time in the meantime and moved all array structures to SlowPutArrayStorage, rematerialization can end up treating rematerialized arrays — now ArrayStorage — as if they had contiguous butterflies. The fix keeps rematerializing with the contiguous butterfly but switches to SlowPutArrayStorage after the fact if the VM is having a bad time.

Source/JavaScriptCore/ftl/FTLOperations.cpp

Source/JavaScriptCore/ftl/FTLOperations.cpp

JSTests/stress/ftl-osr-exit-phantom-new-array-with-butterfly-having-a-bad-time.js

Patch Details

Both production changes sit in the PhantomNewArrayWithButterfly path of FTL OSR-exit object rematerialization in FTLOperations.cpp.

In operationMaterializeObjectInOSR, the structure lookup changes from globalObject->arrayStructureForIndexingTypeDuringAllocation(materialization->indexingType()) to globalObject->originalArrayStructureForIndexingType(materialization->indexingType()). Because the rematerialization code always builds a non-ArrayStorage (contiguous, double, or int32-shaped) butterfly by hand, the structure must be the original structure for that indexing type rather than whatever the global object currently hands out for allocation. A new tail block is added after the butterfly has been sentinel-filled: if globalObject->isHavingABadTime(), the array is converted properly via result->switchToSlowPutArrayStorage(vm), with an ASSERT_ENABLED-only cross-check that arrayStructureForIndexingTypeDuringAllocation would indeed have returned a hasSlowPutArrayStorage structure at that point.

In operationPopulateObjectInOSR, the hole-writing fast path gains a third branch — else if (hasAnyArrayStorage(array->indexingType()) && !value) array->butterfly()->arrayStorage()->m_vector[index].clear(); — handling the case where the array has already been switched to SlowPutArrayStorage, so an empty-JSValue hole must be cleared in the ArrayStorage vector rather than written through contiguous().atUnsafe(index). Comment blocks in both functions were extended; the remaining file change is the new regression test.

Deriving an object's type descriptor from mutable global state while its memory layout is built from a stale compile-time assumption, so the descriptor and the backing store disagree.

Background

Butterfly. JSC stores an object's indexed elements and out-of-line named properties in a single heap block called the butterfly. The pointer points into the middle, with named properties below and indexed elements above, preceded by an IndexingHeader holding public length and vector length.

Indexing types. An array's structure records how the element region is laid out. ArrayWithInt32, ArrayWithDouble, and ArrayWithContiguous store elements starting at the butterfly pointer itself; ArrayWithArrayStorage and ArrayWithSlowPutArrayStorage instead place an ArrayStorage header at the butterfly pointer (m_sparseMap, a write-barriered pointer; m_indexBias; m_numValuesInVector) followed by the element vector m_vector[].

Having a bad time. When script does something that makes fast indexed access unsound for all arrays — most commonly installing an indexed accessor on Array.prototype or Object.prototype — the global object calls haveABadTime(), which converts existing arrays to SlowPutArrayStorage and makes arrayStructureForIndexingTypeDuringAllocation() return a SlowPutArrayStorage structure for every indexing type from then on. originalArrayStructureForIndexingType() returns the pre-bad-time structure for a given indexing type. JSObject::switchToSlowPutArrayStorage() performs the real conversion: it reallocates the butterfly into ArrayStorage shape and migrates the elements.

Allocation sinking. An FTL optimization that removes object allocations whose objects never escape the compiled region. The removed allocation is recorded as a phantom materialization descriptor — PhantomNewArrayWithButterfly here — listing the property and element values held in registers or on the stack.

OSR exit and rematerialization. When optimized code bails out to a lower tier, any sunk object that becomes observable must be reconstructed on the heap. operationMaterializeObjectInOSR allocates the object and its butterfly; operationPopulateObjectInOSR then writes the recorded element values into it.

Hole sentinels. An unwritten element is the empty JSValue for int32/contiguous arrays and PNaN for double arrays; the populate path writes these directly into the butterfly instead of going through putDirectIndex, which would otherwise force an indexing-type conversion.

Watchpoints. FTL code compiled under the assumption that the VM is not having a bad time is invalidated when that assumption is broken, which is one way an OSR exit is forced.

Analysis

This is a type confusion in which the structure and the butterfly are chosen by two lookups separated in time:

  Compile time                     Exit time (bad time now on)
  ────────────                     ───────────────────────────
  sink new Array(5)                structure := arrayStructureFor...
    indexingType = Double            ──► SlowPutArrayStorage
                                   butterfly := hand-built contiguous
                                     elem[0] at butterfly + 0

  ArrayStorage view of that block:
    butterfly + 0  ─► m_sparseMap      ◄── holds 1.1 (attacker double)
    butterfly + 8  ─► m_indexBias/m_numValuesInVector
    butterfly + 16 ─► m_vector[0]      ◄── reads elem[2]

The bad-time transition can occur between FTL compilation of the function and the OSR exit that materializes the sunk array — exactly the window the regression test opens by calling Object.defineProperty(Array.prototype, 0, {get(){}}) inside a noInlined callee in the middle of opt(). On exit, the array is given a structure whose indexing type is ArrayStorage while its butterfly is laid out contiguously. Every subsequent access reads or writes elements at a fixed positive displacement from where they were actually stored, and the first slots of the element data are reinterpreted as the ArrayStorage header — including m_sparseMap, a pointer field.

operationPopulateObjectInOSR compounded it: with the array now typed ArrayStorage, hasDouble / hasInt32 / hasContiguous all fail, so element values are routed through putDirectIndex, which stores into m_vector[index] on a butterfly sized for contiguous storage.

Everything the test does other than the shell helpers (gc(), noInline, --jitPolicyScale) has a plain-JS equivalent: heat a function until the FTL compiles it with the array allocation sunk, then trigger haveABadTime() from inside a call in the middle of that function. Walking it: opt() allocates new Array(5), fills it with doubles so the phantom materialization carries ArrayWithDouble, and never lets the array escape before the final sum, so allocation sinking removes it. On the 1001st call trigger is true, so cb() installs the indexed accessor; the global object enters the bad-time state and the FTL code's assumption is invalidated, forcing the exit that must rematerialize the sunk array. Pre-fix, arrayStructureForIndexingTypeDuringAllocation(ArrayWithDouble) answers with a SlowPutArrayStorage structure while the following code still builds and sentinel-fills the butterfly through butterfly->contiguous().atUnsafe(index). The five recorded values then go through putDirectIndex, and the trailing gc() walks the array with the mismatched view. Whether that write takes the ArrayStorage fast path in-vector or the slow path depends on canSetIndexQuicklyForPutDirect, which consults vector length and m_numValuesInVector and is not in the supplied context.

Projecting from there: because the ArrayStorage header overlays slots the attacker populated with chosen doubles, the m_sparseMap WriteBarrier field could be made to hold a fully controlled bit pattern, and m_numValuesInVector / m_indexBias could be set to chosen 32-bit values. If a subsequent slow-put or sparse-map lookup dereferences that field, an attacker might obtain a fake-object read primitive; if m_indexBias-driven arithmetic is used to recompute the vector base, further displacement of accesses could follow. Both escalations are projected directions, not established behavior: reaching them requires heap grooming so the out-of-bounds writes at the top of m_vector[] land on a chosen adjacent object, plus control over which JS-visible operation first consumes the confused array — neither of which this change establishes.

This vulnerability weakens JSC's type-safety boundary inside the WebContent process: the invariant that a JSObject's structure faithfully describes its butterfly layout. Before the fix, ordinary script could produce a JSArray whose structure says SlowPutArrayStorage while its backing store is contiguous, so every later indexed access — from JS, from inline caches, and from the GC's visit routine — interprets attacker-written element data as ArrayStorage metadata. An attacker who reliably lands this state could obtain out-of-bounds access adjacent to the butterfly plus a pointer field populated from a controlled double, the standard starting point for building arbitrary read/write within the renderer; a further sandbox escape would still be required for anything beyond WebContent.

The interesting shape is that the bug lives in a reconstruction path, not an allocation path. operationMaterializeObjectInOSR has to reproduce, at exit time, an object whose shape was decided at compile time, so it is structurally exposed to any global VM state change in between. arrayStructureForIndexingTypeDuringAllocation() is the correct call for a normal allocation site precisely because it follows the current bad-time state; it is the wrong call anywhere the caller has already committed to a concrete memory layout. The fix's shape is the general remedy: do the layout conversion through the routine that actually rewrites memory, never by swapping the type descriptor. Note also the ripple — fixing the materialize side forced a matching branch in the populate side, because the hole-writing fast paths there had also been written against the assumption that a rematerialized array is never ArrayStorage. Any invariant of the form "this path only ever sees layout X" tends to be duplicated in several functions, and all of them break together.

Audit directions

• Compile-time layout paired with a type descriptor fetched later from mutable global state. The descriptor tracks the world, the layout does not. Narrow: grep Source/JavaScriptCore/ftl/FTLOperations.cpp and Source/JavaScriptCore/dfg/DFGOSRExit*.cpp for every remaining call to arrayStructureForIndexingTypeDuringAllocation, arrayStructureForProfileDuringAllocation, and sibling accessors reached from OSR-exit or rematerialization code, and check each against the other Phantom* materialization cases (PhantomNewArrayBuffer, PhantomNewArrayWithSpread, PhantomCreateRest, PhantomSpread) that also hand-build storage. Code-review tell: the same function both queries a global-object structure accessor and writes through a layout-specific accessor such as contiguous(), contiguousDouble(), or arrayStorage() — if those two facts are not derived from the same source, it's a candidate. Wider: the same class shows up for any deferred object construction whose shape is fixed by an earlier decision — DFG/FTL inline-cache stub generation that caches a structure and later writes storage, and Structure-swapping helpers that change a type tag without reallocating the backing store; look for any setStructure / setStructureIDDirectly not immediately adjacent to the reallocation that justifies it. Widest: a type tag and its backing memory layout must be produced by a single atomic decision, never by two lookups separated in time — applies to V8's Turbofan escape-analysis materialization, SpiderMonkey's bailout object reconstruction, HotSpot's scalar-replacement reallocation, and serialization/deserialization pairs generally.

• Indexing-type dispatch on objects the consumer did not allocate. Narrow: in operationPopulateObjectInOSR and its DFG counterparts, enumerate every hasDouble / hasInt32 / hasContiguous / hasAnyArrayStorage chain that ends in a direct butterfly()->...atUnsafe() write and confirm each chain is exhaustive rather than assuming a closed set of indexing types — the added hasAnyArrayStorage(...) && !value branch exists precisely because the previous chain silently fell through to putDirectIndex for an unanticipated type. Code-review tell: an if/else-if ladder over has*(indexingType()) whose final else performs a semantically different operation rather than handling the residual cases. Wider: the same shape appears anywhere WebKit dispatches on a tag with an assumed-closed value set — JSType switches in the GC's visitChildren, ArrayMode handling in DFG fixup, IndexingType switches in JSObject conversion helpers; look for switches without a default: RELEASE_ASSERT_NOT_REACHED(). Widest: any dispatch that enumerates a tag's cases must either cover them all or hard-fail on the residual — Rust match arms collapsed into _ =>, C switch without default, protobuf oneof handling.

• Global VM state changing inside the compile-to-exit window. Narrow: trace the callers of JSGlobalObject::haveABadTime() and the watchpoint sets that guard array fast paths (havingABadTimeWatchpoint, arrayIteratorProtocolWatchpointSet, prototype-chain structure watchpoints) and check which exit-time runtime helpers in Source/JavaScriptCore/ftl/ and Source/JavaScriptCore/dfg/ read state those watchpoints protect after an exit has begun. Match tell: an exit-time helper reading a JSGlobalObject accessor whose value is watchpoint-protected in compiled code — the watchpoint no longer protects anything once the code has been invalidated. Wider: generalize to any "invalidated assumption survives into the recovery path" case — deferred destructor and finalizer callbacks, and the sentinel/scratch-buffer contents used by DFG exit recovery, which are likewise written under compile-time assumptions and consumed later. Widest: a recovery path must not consume assumptions that the very event triggering recovery has just falsified — applies to any speculate-and-deoptimize system, including database query-plan invalidation.

• Hole and sentinel handling across layout conversions. Narrow: examine every site that writes a hole sentinel directly into a butterfly — setStartingValue(JSValue()) for int32/contiguous, PNaN for double, .clear() for ArrayStorage vectors — in FTLOperations.cpp and the JSObject.cpp conversion helpers (convertContiguousToArrayStorage, switchToSlowPutArrayStorage and friends), and check whether the sentinel written matches the indexing type the object holds at the moment of the write rather than at the start of the enclosing function. Code-review tell: a hole write whose layout accessor is selected by a condition evaluated earlier in the function than the write itself. Wider: sentinel encodings that differ per representation, written by code that cached the representation, also covers m_numValuesInVector bookkeeping and sparse-map transitions. Widest: when the same logical value ("absent") has representation-dependent encodings, the encoding choice must be re-derived at the point of the write — hash-table tombstones, columnar-storage null encodings, tagged-pointer NaN boxing in other engines.