Bug 312685 – Set Spread in DFG/FTL Missing Per-Instance Prototype Check

• Component: JavaScriptCore
• Status: RESOLVED FIXED

Created attachment 479201 [details]
poc.js

## Summary

The DFG/FTL JIT compilers produce incorrect output for `[...set]` when the operand's prototype has been replaced (e.g., `Object.setPrototypeOf(set, {})`). The interpreter correctly throws `TypeError` because `Symbol.iterator` is no longer reachable; the JIT silently reads internal Set storage and returns its values.

**While I'm inclined to think that this is non-security bug, I thought it would be safer to report this as security. Feel free to demote this as a bug.**

## Bug

### Summary

The DFG fixup phase (`DFGFixupPhase.cpp:1993-1996`) enables the `SetObjectUse` fast path on two global conditions only: the `SetIteratorProtocolWatchpoint` is valid and `HavingABadTime` is valid. Neither watchpoint fires when `Object.setPrototypeOf` is called on an individual instance, so the JIT fast path is taken for sets that no longer have a reachable `Symbol.iterator`. The per-instance check — `JSSet::isIteratorProtocolFastAndNonObservable()` — exists and correctly handles this, but is never invoked anywhere in the DFG/FTL Set spread path, nor in the `operationSpreadSet` slow-path fallback.

### Detail

**DFG fixup condition** (`DFGFixupPhase.cpp:1993-1996`):

```cpp
else if (node->child1()->shouldSpeculateSetObject()
    && m_graph.isWatchingSetIteratorProtocolWatchpoint(node->child1().node())
    && m_graph.isWatchingHavingABadTimeWatchpoint(node->child1().node()))
    fixEdge<SetObjectUse>(node->child1());
```

`SetIteratorProtocolWatchpoint` only fires when `Set.prototype[Symbol.iterator]` or `SetIterator.prototype.next` are modified globally — it is not invalidated by per-instance prototype changes. Compare the array spread path just above (lines 1988–1991), which has an additional `isWatchingArrayPrototypeChainIsSaneWatchpoint()` guard; the Set path has no equivalent.

**`operationSpreadSet` has no per-instance check** (`DFGOperations.cpp:4843-4853`):

```cpp
JSC_DEFINE_JIT_OPERATION(operationSpreadSet, JSCell*, (JSGlobalObject* globalObject, JSCell* cell))
{
    ...
    ASSERT(jsDynamicCast<JSSet*>(cell));
    JSSet* set = jsCast<JSSet*>(cell);
    OPERATION_RETURN(scope, JSCellButterfly::createFromSet(globalObject, set));
}
```

The array counterpart `operationSpreadFastArray` (`DFGOperations.cpp:4856-4867`) has:

```cpp
    ASSERT(array->isIteratorProtocolFastAndNonObservable()); // absent in operationSpreadSet
```

The missing function (`JSSetInlines.h:37-56`) correctly validates the instance:

```cpp
ALWAYS_INLINE bool JSSet::isIteratorProtocolFastAndNonObservable()
{
    JSGlobalObject* globalObject = this->realm();
    if (!globalObject->isSetPrototypeIteratorProtocolFastAndNonObservable())
        return false;
    Structure* structure = this->structure();
    if (structure == globalObject->setStructure())
        return true;
    if (getPrototypeDirect() != globalObject->jsSetPrototype())
        return false;  // catches Object.setPrototypeOf(set, {}) / null
    if (getDirectOffset(vm, vm.propertyNames->iteratorSymbol) != invalidOffset)
        return false;  // catches set[Symbol.iterator] = customFn
    return true;
}
```

The same fast-path inline code appears in the FTL at `FTLLowerDFGToB3.cpp:10312`.

### Trigger Conditions

1. A function containing `[...set]` is compiled to DFG or FTL
2. `SetIteratorProtocolWatchpoint` is valid (global `Set.prototype` is unmodified)
3. A `JSSet` instance has its prototype replaced via `Object.setPrototypeOf` or owns a `Symbol.iterator` property
4. The modified instance is passed to the compiled function

## Version

### Reproduced Version

- `main` branch latest commit (2026/04/19): `a4390137a403`

## Reproduction Case

### Release Build

```bash
WebKitBuild/JSCOnly/Release/bin/jsc poc.js
```

```
interp: TypeError: Spread syntax requires ...iterable[Symbol.iterator] to be a function
jit: [1,2,3]
```

Debug build produces identical output with no ASSERT fired, confirming `operationSpreadSet` lacks the invariant check present in `operationSpreadFastArray`.

### PoC Code

```js
function spreadSet(s) { return [...s]; }

// Baseline: interpreter must throw TypeError (no Symbol.iterator in prototype chain)
let cold = new Set([1, 2, 3]);
Object.setPrototypeOf(cold, {});
try { [...cold]; print("interp: no error"); } catch (e) { print("interp: " + e); }

// Warm up spreadSet to DFG/FTL
for (let i = 0; i < 200000; i++) spreadSet(new Set([1, 2, 3]));

// JIT: same input — must throw TypeError, but returns values instead
let hot = new Set([1, 2, 3]);
Object.setPrototypeOf(hot, {});
try { let r = spreadSet(hot); print("jit: " + JSON.stringify(r)); }
catch (e) { print("jit: " + e); }
```

## Suggested Patch

Verified: applying this patch fixes the bug (both lines print `TypeError`); reverting restores it.

```diff
diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
@@ -4850,6 +4850,19 @@ JSC_DEFINE_JIT_OPERATION(operationSpreadSet, JSCell*, (JSGlobalObject* globalObj
     ASSERT(jsDynamicCast<JSSet*>(cell));
     JSSet* set = jsCast<JSSet*>(cell);
 
+    if (!set->isIteratorProtocolFastAndNonObservable()) {
+        JSFunction* iterationFunction = globalObject->iteratorProtocolFunction();
+        auto callData = JSC::getCallData(iterationFunction);
+        ASSERT(callData.type != CallData::Type::None);
+        MarkedArgumentBuffer arguments;
+        arguments.append(JSValue(set));
+        ASSERT(!arguments.hasOverflowed());
+        JSValue arrayResult = call(globalObject, iterationFunction, callData, jsNull(), arguments);
+        OPERATION_RETURN_IF_EXCEPTION(scope, nullptr);
+        JSArray* array = jsCast<JSArray*>(arrayResult);
+        OPERATION_RETURN(scope, JSCellButterfly::createFromArray(globalObject, vm, array));
+    }
+
     OPERATION_RETURN(scope, JSCellButterfly::createFromSet(globalObject, set));
 }
 
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -9392,6 +9392,14 @@ void SpeculativeJIT::compileSpread(Node* node)
 
         using Helper = JSSet::Helper;
 
+        // Guard: Object.setPrototypeOf and own-property additions both cause structure
+        // transitions, so a mismatched StructureID means the iterator protocol may have
+        // changed on this instance. Route such sets to the slow path.
+        JSGlobalObject* globalObject = m_graph.globalObjectFor(node->origin.semantic);
+        load32(Address(argument, JSCell::structureIDOffset()), scratch2GPR);
+        slowPath.append(branch32(NotEqual, scratch2GPR,
+            TrustedImm32(m_graph.registerStructure(globalObject->setStructure())->id().bits())));
+
         // Load Set storage pointer.
         loadPtr(Address(argument, JSSet::offsetOfStorage()), scratch1GPR);
         slowPath.append(branchTestPtr(Zero, scratch1GPR));
 
diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
@@ -10312,6 +10312,7 @@ IGNORE_CLANG_WARNINGS_END
         } else if (m_node->child1().useKind() == SetObjectUse) {
             using Helper = JSSet::Helper;
 
+            LBasicBlock structureCheck = m_out.newBlock();
             LBasicBlock obsoleteCheck = m_out.newBlock();
             LBasicBlock deletedCheck = m_out.newBlock();
             LBasicBlock sizeCheck = m_out.newBlock();
@@ -10320,12 +10321,20 @@ IGNORE_CLANG_WARNINGS_END
             LBasicBlock slowPath = m_out.newBlock();
             LBasicBlock continuation = m_out.newBlock();
 
+            // Guard: route prototype-mutated or own-Symbol.iterator Sets to slowPath.
+            LValue structureID = m_out.load32(argument, m_heaps.JSCell_structureID);
+            m_out.branch(m_out.notEqual(structureID,
+                weakStructureID(m_graph.registerStructure(globalObject->setStructure()))),
+                rarely(slowPath), usually(structureCheck));
+
+            LBasicBlock lastNext = m_out.appendTo(structureCheck, obsoleteCheck);
+
             // Load Set storage pointer.
             LValue storage = m_out.loadPtr(argument, m_heaps.JSSet_storage);
             m_out.branch(m_out.isNull(storage), rarely(slowPath), usually(obsoleteCheck));
 
             // Check storage is not obsolete (slot 0 must be Int32).
-            LBasicBlock lastNext = m_out.appendTo(obsoleteCheck, deletedCheck);
+            m_out.appendTo(obsoleteCheck, deletedCheck);
             LValue storageButterfly = toButterfly(storage);
```

### Credit Information

Reporter credit: Junyoung Park(@candymate) of KAIST Hacking Lab

[Radar WebKit Bug Importer]

<rdar://problem/175083041>

[Shu-yu Guo]

This is a correctness bug, not a security one.

[Kai Tamkun]

Pull request: https://github.com/WebKit/WebKit/pull/64218

[Kai Tamkun]

Pull request: https://github.com/apple/WebKit/pull/5236

[EWS]

Committed 313031@main (3876c27e9c01): <https://commits.webkit.org/313031@main>

Reviewed commits have been landed. Closing PR #64218 and removing active labels.