2045321 - StubFolding.cpp: AddToFoldedStub accepts strict-prefix CacheIR streams, poisoning the folded stub's ShapeList (JIT type confusion, forged iterator.return call)

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Christian Holler (:decoder)]

Summary

The CacheIR stub-folding machinery can be tricked into appending an arbitrary object shape to the ShapeListObject / ShapeListWithOffsetsObject (js/src/jit/ShapeList.cpp) of an already-folded IC stub, even though the folded stub's loads, guards and call were never validated for that shape.

The shapes stored in these lists are the entire security boundary of a folded stub: once a shape is in the list, GuardMultipleShapes / GuardMultipleShapesToOffset accepts any object with that shape and the rest of the folded stub executes raw slot loads/stores at baked offsets against it. Poisoning the list yields:

• out-of-bounds / type-confused slot reads (LoadDynamicSlot on an object that has no dynamic slots reads past the shared emptyObjectSlots sentinel or past a smaller slots allocation), and
• a forged iterator.return() method call: the confused slot load is treated as the iterator's return method and invoked.

This is reachable from plain web JavaScript (no shell-only features required).

Root cause

js::jit::AddToFoldedStub (js/src/jit/StubFolding.cpp) decides whether a newly generated stub is "the same stub except for the shape (+offset)" by walking the new stub's CacheIR stream and the folded stub's stream in lockstep:

  CacheIRReader stubReader(stubInfo);
  CacheIRReader newReader(writer);
  while (newReader.more() && stubReader.more()) {   // <-- exits on EITHER end
    ...
  }
  ...
  shapeList->append(cx, newShape);                  // poisons the list

There is no check that both streams were fully consumed. If the new stub's stream is a strict prefix of the folded stub's stream, the loop exits after matching only the prefix and all trailing ops of the folded stub (loads, callee guards, the call itself) are accepted unchecked. The final stub-data comparison CacheIRWriter::stubDataEqualsIgnoringShapeAndOffset (js/src/jit/CacheIRCompiler.cpp:1497) iterates only the new writer's fields, so the folded stub's trailing stub fields are not compared either.

This prefix acceptance became reachable when ReturnFromIC was removed (bug 2041861, commit 94608d120c93): previously every CacheIR stream ended with a ReturnFromIC op, so a shorter stream's terminator necessarily mismatched the folded stub's mid-stream op. That commit only weakened two debug asserts (peekOp() == ReturnFromIC -> !more(), StubFolding.cpp:698-699) and added no completeness check. The validation gap itself dates to the introduction of the AddToFoldedStub stream matching (bug 1999828).

Weaponization via CloseIter

CloseIterIRGenerator::tryAttachNoReturnMethod (js/src/jit/CacheIR.cpp:16704) emits a guard-only stream; for a null-prototype object it is exactly one op:

GuardShape obj0, <field0: WeakShape>

A folded tryAttachScriptedReturn stub (own return data property in a dynamic slot) looks like:

GuardMultipleShapes   obj0, <field0: JSObject shapeList>
LoadDynamicSlot       res1, obj0, <field1: RawInt32 slotIndex K>
GuardToObject         1
GuardFunctionScript   1, <field2: BaseScript>, <field3: RawInt32 nargsAndFlags>
CloseIterScriptedResult obj0, 1, nargs

The 1-op stream prefix-matches: iteration 1 hits the CacheOp::GuardMultipleShapes case (StubFolding.cpp:527), which matches GuardShape (equal operand ids, field offsets, same realm); then newReader.more() is false and the loop exits. The evil shape is appended to the folded stub's ShapeListObject. From then on, every for...of early exit over an object with that shape runs LoadDynamicSlot(evil, K) — a type-confused (potentially OOB) read — and calls the loaded value as the return method.

Note: the same missing-length check also allows the reverse direction (new stream longer than the folded stream), where stubDataEqualsIgnoringShapeAndOffset reads past the folded stub's stub data (heap over-read in the comparison itself). Separately, that function's continue for the offset field fails to advance the comparison cursor, so all fields after the offset field are skipped — a second latent validation gap that should be fixed alongside.

Reproduction

Testcase: poc.js (attached). Run:

js --fuzzing-safe poc.js

100% reproducible (20/20 runs) on both an ASan debug build and an ASan fuzzing-opt build of mozilla-central, version 20260603-5cffec5992d3 (from the shell's .fuzzmanagerconf product_version).

The testcase is differential: closing an iterator that has no return method must call nothing.

• Buggy: js --fuzzing-safe poc.js throws Error: SECURITY BUG: CloseIter on an iterator without a return method invoked a forged function (1 bogus calls) via a type-confused slot load (exit 3).
• Correct: js --fuzzing-safe --no-jit-backend poc.js prints ok (exit 0); V8 also prints ok.

The gc() call in the PoC is optional (guarded by typeof) and only makes stub folding more reliable; it is not required for the bug.

PoC outline:

1. Build 8 iterator objects with distinct shapes, each with an own return function in the same dynamic slot K, and warm up a single for...of + break site (JSOp::CloseIter) so TryFoldingGuardShapes folds the stubs into one GuardMultipleShapes stub over a ShapeListObject.
2. Close a null-prototype iterator without any return property. Its stub is the 1-op [GuardShape] stream, which prefix-matches in AddToFoldedStub and appends its shape to the folded stub's shape list.
3. Close it again: the folded stub now matches, performs the confused LoadDynamicSlot, and calls the forged return method.

Suggested fix

In AddToFoldedStub, after the matching loop, fail unless both streams were fully consumed:

  if (newReader.more() || stubReader.more()) {
    return false;
  }

Additionally fix stubDataEqualsIgnoringShapeAndOffset so that skipping the offset field does not also skip every subsequent field (track the two layout cursors separately), and consider re-adding a release-mode guarantee that folded stubs and candidate stubs agree on stream length now that ReturnFromIC no longer terminates streams.

Affected code

• js/src/jit/StubFolding.cpp — AddToFoldedStub (missing completeness check; appends to the shape lists).
• js/src/jit/ShapeList.cpp — ShapeListObject / ShapeListWithOffsetsObject, the poisoned guard lists.
• js/src/jit/CacheIRCompiler.cpp — stubDataEqualsIgnoringShapeAndOffset (cursor bug, trailing fields never compared).
• Regressed by: bug 2041861 (94608d120c93, "Remove ReturnFromIC").

Comment 1

[Christian Holler (:decoder)]

Attachment: poc.js — testcase (run with js --fuzzing-safe poc.js)

Comment 2

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 2041861

:iain, since you are the author of the regressor, bug 2041861, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: (secure)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b3e803f64871
https://hg.mozilla.org/integration/autoland/rev/b2e591cb54d5
Fix CacheIR reading in AddToFoldedStub. r=mgaudet

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/b2e591cb54d5