b67282ad89  [JSC] Missing exception check in `forEachInIteratorProtocol` Set iterator fast path

The bug at a glance

OBSERVED: the fix adds two RETURN_IF_EXCEPTION guards to the Set-iterator fast path in forEachInIteratorProtocol and ships a regression test that runs under –validateExceptionChecks=1. INFERRED: the direct consequence is invoking a user (JS) callback while a JSC exception is pending. In JSC, running arbitrary JS with an unchecked pending exception violates the engine’s exception-check invariant; on debug/ASan builds this fires an assertion, and on release builds it can lead to executing code paths that assume no exception is set. That is a correctness/soundness defect in a hot iteration primitive reachable from ordinary script (Set.prototype.entries() plus any Iterator-helper consumer), justifying a medium-to-high rating; it is rated medium here because the throwing condition (constructArrayPair OOM for kind==Entries) is not trivially attacker-triggerable without exhausting memory.

A newly-added Set-iterator fast path in forEachInIteratorProtocol copied the shape of the Map-iterator path but dropped the exception check that must sit between iterator next() and the user callback. Because JSSetIterator::next() can throw (OOM while building the [key,value] pair for entries kind), the callback could be entered with a pending exception.

Root cause

forEachInIteratorProtocol is a JSC-internal helper (Source/JavaScriptCore/runtime/IteratorOperations.h) used to drive the iterator protocol from C++ for Iterator-helper style consumers such as Iterator.prototype.forEach and Iterator.prototype.toArray. It has fast paths that recognize when the iterable is a built-in Set or Map iterator (setIterator->iteratedObject() / the Map equivalent) and drive it directly via JSSetIterator::next(globalObject, value) rather than through the generic, slower iterator dispatch. This avoids per-step property lookups of the iterator’s next method.

When a Set iterator’s kind is Entries, each step must produce a two-element array [value, value]. JSSetIterator::next() constructs that pair via constructArrayPair, which allocates a JSArray. Allocation can fail and throw an out-of-memory exception; more generally next() is declared as a call that may set a pending exception on the VM. The correct pattern, which the Map-iterator branch already followed, is: call next(), immediately RETURN_IF_EXCEPTION, then invoke the user callback, then RETURN_IF_EXCEPTION again after the callback (which itself runs arbitrary JS).

OBSERVED: the Set fast path, added in 314169@main (referenced in the commit message), was missing the check between next() and callback(vm, globalObject, value), and also lacked the trailing check after the while loop exits. The patch inserts RETURN_IF_EXCEPTION(scope, void()) immediately after the while (setIterator->next(…)) condition and a second one before the return, mirroring the Map branch so both are consistent.

INFERRED: without the first check, if next() threw (constructArrayPair OOM on an Entries iterator), the loop condition would still evaluate and control would fall into callback(…), invoking a JavaScript function while scope holds a pending exception. JSC maintains an exception-check invariant: every point that can observe or clear an exception must be checked before further JS executes. Calling into JS with an unhandled pending exception is precisely what –validateExceptionChecks=1 is designed to catch, and it asserts ’exception check validation failed’ on debug/ASan builds. On release builds the callback would execute against inconsistent VM state (an exception logically thrown but the throw not yet propagated), which is undefined-behavior-prone reentry rather than a memory-corruption primitive per se.

Key code

The added exception checks bracketing the user callback in the Set-iterator fast path (IteratorOperations.h)

            if (setIterator->iteratedObject()) {
                JSValue value;
                while (setIterator->next(globalObject, value)) {
                    RETURN_IF_EXCEPTION(scope, void());
                    callback(vm, globalObject, value);
                    RETURN_IF_EXCEPTION(scope, void());
                }
                RETURN_IF_EXCEPTION(scope, void());
                return;
            }

Patch walkthrough

• Source/JavaScriptCore/runtime/IteratorOperations.h — In the Set-iterator fast path of forEachInIteratorProtocol, adds RETURN_IF_EXCEPTION(scope, void()) immediately after setIterator->next(globalObject, value) returns true and before callback() is invoked, so a throw from next() (e.g. constructArrayPair OOM for the Entries kind) aborts before running the user callback. Adds a second RETURN_IF_EXCEPTION before the function returns, matching the existing Map-iterator branch exactly.
• JSTests/stress/iterator-helpers-set-entries-exception-check.js — New regression test run with –validateExceptionChecks=1. Exercises Set.prototype.entries() through Iterator.prototype.forEach (JS callback and bound non-JS callback) and toArray, plus a Map entries case for symmetry and a 1000-iteration loop to cover JIT tiers. On an unpatched build the missing check trips the exception-validation assertion when the JS callback runs.

Background

forEachInIteratorProtocol — A JSC C++ helper in IteratorOperations.h that drives the iterator protocol and invokes a supplied callback per element, with fast paths that special-case built-in Set and Map iterators to avoid generic next-method dispatch.

JSSetIterator::next() / constructArrayPair — The Set iterator step function. For kind==Entries it allocates a two-element array via constructArrayPair, an allocation that can throw OutOfMemory, so next() is an exception-emitting call.

RETURN_IF_EXCEPTION — JSC macro that checks the ThrowScope for a pending exception and returns immediately if one is set. Mandatory after any call that may throw and before executing further JS.

–validateExceptionChecks — A JSC debug/ASan validation mode that asserts if the engine reaches an exception-observing point (like calling into JS) without having checked a pending exception since the last throwable call. The regression test relies on it.

Exception-check invariant — JSC’s rule that arbitrary JS must never be entered while an exception is pending-but-unchecked; violating it leaves the VM in an inconsistent state and is the class of bug this patch closes.

Vulnerability window

1. Baseline — The Map-iterator fast path in forEachInIteratorProtocol includes the RETURN_IF_EXCEPTION guards around the callback.
2. Regression introduced — Change 314169@main adds a parallel Set-iterator fast path but omits the exception check between next() and callback and the trailing check after the loop.
3. Latent defect — For a Set entries iterator, a throw from next() (constructArrayPair OOM) would fall through into the JS callback with a pending exception.
4. Detection — The condition is exactly what –validateExceptionChecks=1 flags; an entries-iterator + Iterator-helper consumer surfaces it as an assertion failure on debug/ASan.
5. Fix (314249@main) — Two RETURN_IF_EXCEPTION(scope, void()) statements added to make the Set branch consistent with the Map branch.
6. Verification — Regression test iterator-helpers-set-entries-exception-check.js added, running under the validation flag across interpreter and JIT tiers.

Proof of concept

VERBATIM excerpt of the added regression test JSTests/stress/iterator-helpers-set-entries-exception-check.js. It must be run with –validateExceptionChecks=1 (the //@ runDefault directive). It drives a Set entries iterator through Iterator.prototype.forEach and toArray; on an unpatched build, once next() throws (OOM building the pair) the JS callback is entered with a pending exception and the validation assertion fires. Reliably forcing the OOM in next() requires memory pressure, so this is primarily a soundness/assertion regression test rather than a corruption exploit.

//@ runDefault("--validateExceptionChecks=1")

function shouldBe(actual, expected) {
    if (actual !== expected)
        throw new Error(`bad value: ${actual}, expected: ${expected}`);
}

// Set entries iterator + Iterator.prototype.forEach (JS callback).
{
    const set = new Set([1, 2, 3]);
    const seen = [];
    set.entries().forEach((entry) => {
        seen.push(entry[0], entry[1]);
    });
    shouldBe(seen.join(","), "1,1,2,2,3,3");
}

// Set entries iterator + Iterator.prototype.toArray.
{
    const set = new Set([1, 2, 3]);
    const result = set.entries().toArray();
    shouldBe(JSON.stringify(result), "[[1,1],[2,2],[3,3]]");
}

Exploitation

1. Trigger surface — Reach the Set-iterator fast path from script via set.entries() consumed by an Iterator-helper (forEach/toArray) or any consumer routed through forEachInIteratorProtocol.
2. Force the throw — Cause JSSetIterator::next() to throw for kind==Entries, i.e. make constructArrayPair fail allocation via memory exhaustion; this is the only throwing path noted, and it is not cheaply attacker-controlled.
3. Observable effect — The user callback runs with a pending exception. On debug/ASan this is an assertion; on release it is inconsistent VM state entering JS, not a demonstrated memory-corruption primitive.
4. Escalation assessment — No direct type confusion or OOB is shown by the patch; exploitability beyond a crash/soundness violation is unestablished and would depend on downstream behavior of running JS under an unhandled exception.

Detection & hunting

For defenders and SOC / detection engineers:

• Assertion under exception-check validation — Run the iterator/Iterator-helper suites with –validateExceptionChecks=1 on debug/ASan; an ’exception check validation failed’ at forEachInIteratorProtocol indicates the missing guard.
• OOM-in-iteration crashes — Crash reports where a JS callback frame is entered right after a Set entries next() under memory pressure point at this class of bug.
• Fast-path asymmetry — Static review flag: any built-in-iterator fast path where next() is followed directly by a callback without an intervening RETURN_IF_EXCEPTION.

Audit directions

• Other forEachInIteratorProtocol fast paths — Audit every specialized iterator branch (arrays, generic iterators) in IteratorOperations.h for the next()->check->callback->check pattern; the Set branch shows the pattern is easy to copy incorrectly.
• Callers of JSSetIterator::next / JSMapIterator::next — Enumerate all C++ call sites of these next() methods and confirm each performs RETURN_IF_EXCEPTION before touching the produced value or calling out.
• constructArrayPair callers — Review all users of constructArrayPair (and similar allocating helpers inside iterator steps) to ensure the throwing allocation is checked before further JS or object use.
• Iterator-helper C++ surface — Audit the recently-added Iterator-helper implementations for exception-check completeness, since 314169@main-era code introduced this regression.