5099a4a895  [JSC] Incorrect loop condition in ClonedArguments::copyToArguments with non-zero offset

The bug at a glance

OBSERVED: a one-line change to a loop bound in ClonedArguments::copyToArguments, a routine that materializes an argument object’s elements into a caller-supplied JSValue* buffer. INFERRED: with a non-zero offset the loop terminated |offset| iterations early, leaving destination slots either unwritten (uninitialized JSValue leaking into a spread/apply target) or, depending on how firstElementDest and the requested length were sized, allowing writes to be miscounted. That is a memory-safety defect in argument marshalling, hence high. Tempering it: the author states there is no known path to reach copyToArguments with a non-zero offset (op_call_varargs did not yield a PoC), which is why the input’s confidence is 0.65 and no test was added.

The bug is a classic off-by-offset in a fixed-vs-copy loop. copyToArguments walks two ranges: a fast prefix copied from an inline/stored region, then a slow tail fetched via get(). The tail loop compared the raw induction variable i against length instead of against length + offset, so when offset != 0 the destination index i - offset never reached the last (length-1) logical element. The interesting question for a researcher is whether any caller can actually supply a non-zero offset with attacker-controlled length; the committer could not construct one, so this is a latent hardening fix rather than a live exploit.

Root cause

ClonedArguments::copyToArguments(globalObject, firstElementDest, offset, length) is the CopyToArguments primitive used when a ClonedArguments object is spread into a call frame or otherwise unpacked into a contiguous JSValue* destination. The parameter offset is the logical index within the arguments object at which copying should begin, and length is the count of elements to copy; the destination slot for logical index i is firstElementDest[i - offset].

The function copies in two phases. A first loop handles the range that can be read directly from the object’s fast storage, writing firstElementDest[i - offset] = value for each i it covers and advancing the shared induction variable i. A second loop then handles any remaining logical indices by calling the general get(globalObject, i) accessor (which honors overridden indexed properties, the length getter, etc.). The induction variable i is shared across both phases.

The defect is in the tail loop’s termination test. Pre-patch it read for (; i < length; ++i). Because i is a logical arguments index that started at offset (not 0), the correct number of elements to produce is length values spanning logical indices [offset, offset + length). Comparing i < length stops the loop once i reaches length, which is offset iterations too early whenever offset > 0. The result is that the final offset destination slots firstElementDest[length - offset .. length) are never assigned by the get() path, and the corresponding high logical indices are never read.

The patch changes the bound to i < length + offset, so the loop runs for the full logical range [.., offset + length) and every intended destination slot receives a value. OBSERVED from the commit message: the author could find no path that invokes copyToArguments with offset != 0, and various attempts to drive it through op_call_varargs failed, so the practical impact is that of an uninitialized/short-write bug that is presently unreachable but would be memory-unsafe if a caller ever passed a non-zero offset with attacker-influenced length.

Key code

Corrected tail-loop bound in ClonedArguments::copyToArguments

            firstElementDest[i - offset] = value;
        }
-        for (; i < length; ++i) {
+        for (; i < length + offset; ++i) {
            firstElementDest[i - offset] = get(globalObject, i);
            RETURN_IF_EXCEPTION(scope, void());
        }

Patch walkthrough

• Source/JavaScriptCore/runtime/ClonedArguments.cpp — In ClonedArguments::copyToArguments the tail loop that fills remaining slots via get(globalObject, i) had its termination condition changed from i < length to i < length + offset. Since i is a logical arguments index beginning at offset and the destination index is i - offset, the old bound stopped offset iterations early, leaving the top destination slots unwritten. The new bound covers the full logical range [offset, offset + length).

Background

ClonedArguments — A JSC arguments-object variant produced when a function’s arguments object must be a genuine, mutable Array-like snapshot (e.g. after the parameters are observed). It stores element values and supports the CopyToArguments protocol used when spreading it into another call.

copyToArguments / offset — Copies length element values into a caller-provided JSValue* buffer firstElementDest, starting at logical index offset. Destination slot for logical index i is firstElementDest[i - offset]. Non-zero offset means the first offset logical elements are skipped.

op_call_varargs — The bytecode/DFG path that materializes a spread or Function.prototype.apply argument list into a fresh call frame. It is the primary consumer of CopyToArguments-style routines and the candidate route the author probed for a non-zero-offset caller.

get(globalObject, i) — The generic indexed accessor used by the tail loop to fetch logical index i, honoring any overridden indexed properties. It can throw, hence the RETURN_IF_EXCEPTION guard on each iteration.

Vulnerability window

1. Introduction — copyToArguments was written with a two-phase copy sharing induction variable i; the tail loop’s bound was set to length rather than length + offset, correct only for the offset == 0 case that all existing callers use.
2. Latency — Because every in-tree caller passes offset == 0, the wrong bound never manifested; JSTests exercise only the zero-offset path.
3. Discovery — Audit (bug 309185 / rdar://171157543) noticed the induction variable is a logical index offset by offset while the bound was the raw count, i.e. an off-by-offset short write.
4. PoC attempt — The author tried to reach a non-zero offset via op_call_varargs and could not build a working trigger.
5. Fix — Bound changed to i < length + offset; no test added because the vulnerable path is not reachable from script today.

Triggering

No test was added. Per the commit message, no script path reaches ClonedArguments::copyToArguments with a non-zero offset; attempts via op_call_varargs failed. Trigger conditions: a caller would have to invoke copyToArguments with offset > 0 and a length such that the logical range [offset, offset+length) exceeds the range covered by the fast prefix loop, causing the top offset destination slots to be left uninitialized (or the copy short by offset elements). Reproduction would require a JSC-internal call site or engine modification that passes a non-zero offset.

Exploitation

1. Reach — An attacker would first need a script-reachable call site that forwards a non-zero offset into copyToArguments; none is currently known, which is the gating obstacle.
2. Short write / uninitialized read — If reached, the top offset destination JSValue slots are never written, so a subsequent consumer (spread target, apply frame) reads whatever garbage/stale JSValue occupies those slots.
3. Type confusion — A stale or uninitialized JSValue interpreted as a boxed pointer is the usual springboard from an uninitialized-slot bug to a fake-object / addrof primitive in JSC.

Detection & hunting

For defenders and SOC / detection engineers:

• Uninitialized JSValue in spread targets —
• Assertion on copy count —

Audit directions

• CopyToArguments family —
• Callers passing offset —
• Spread/apply lowering —