Bug: Bug 2020906
Changeset: 81f2dd7c9d474e765bf167801120034e0f4ef082
Commit message: "Bug 2020906 - Improve FoldTests pattern matching. r=iain"
Reporter: maxpl0it working with Trend Micro Zero Day Initiative
File changed: js/src/jit/IonAnalysis.cpp
Affected pass: FoldTests - a MIR optimization pass that folds conditional test chains (diamond and triangle patterns)
The patch modifies the FoldTests pass in js/src/jit/IonAnalysis.cpp. This pass optimizes chains of conditional tests (MTest instructions) by identifying:
- Diamond patterns:
if (cond) { A } else { B }; test(phi(A,B))- folds the phi + second test into a single test - Triangle patterns:
if (cond) { A }; test(phi(A, original))- similar fold with only one branch
The specific functions changed are:
BlockIsSingleTest()- validates a block is just a phi + testUpdateGotoSuccessor()- rewires goto targets during foldingUpdateTestSuccessors()- rewires test true/false targets during foldingIsDiamondPattern()- detects diamond CFG shapesIsTrianglePattern()- detects triangle CFG shapes- The main FoldTests loop that applies these patterns
The fix makes two critical categories of changes:
Multiple debug-only assertions were upgraded to release assertions. This is the exploit-relevant change. In debug builds, MOZ_ASSERT fires and crashes. In release builds (what users run), MOZ_ASSERT is compiled out entirely - the condition is never checked. By upgrading to MOZ_RELEASE_ASSERT, the checks now run in production.
The critical assertions that were debug-only:
// BEFORE (vulnerable - release builds skip these):
MOZ_ASSERT(ins->isGoto()); // assumed block ends in goto
MOZ_ASSERT(test->input() == value); // assumed test input matches
MOZ_ASSERT(phi->numOperands() == 2); // assumed phi has exactly 2 inputs
MOZ_ASSERT(test->ifFalse() == phiBlock); // assumed false branch is phiBlock
MOZ_ASSERT(!pred->isLoopBackedge()); // assumed not a loop backedge
MOZ_ASSERT(phiBlock->numPredecessors() + 1 == oldNumPred); // assumed progress
// AFTER (fixed - release builds check these):
MOZ_RELEASE_ASSERT(ins->isGoto());
MOZ_RELEASE_ASSERT(test->input() == value);
MOZ_RELEASE_ASSERT(phi->numOperands() == 2);
MOZ_RELEASE_ASSERT(test->ifFalse() == phiBlock);
MOZ_RELEASE_ASSERT(!pred->isLoopBackedge());
MOZ_RELEASE_ASSERT(phiBlock->numPredecessors() + 1 == oldNumPred);The old code used block->getSuccessor(0) to navigate the CFG. This assumes the block's first successor is always the goto target. But getSuccessor(0) returns the first successor in the successor list, which may not be the goto target if the block ends in an MTest (which has two successors: true branch at index 0, false branch at index 1).
// BEFORE (vulnerable):
MBasicBlock* phiBlock = trueBranch->getSuccessor(0);
// If trueBranch ends in MTest (not MGoto), getSuccessor(0) returns
// the TRUE branch, not necessarily the phi block.
// AFTER (fixed):
MBasicBlock* phiBlock = trueBranch->lastIns()->toGoto()->target();
// Explicitly checks that the last instruction IS a goto,
// then gets its target. If it's not a goto, toGoto() crashes
// via MOZ_RELEASE_ASSERT.// BEFORE (vulnerable):
if (testBlock->numSuccessors() == 1) {
testBlock = testBlock->getSuccessor(0);
}
// A block with numSuccessors == 1 could be a goto OR could be
// a degenerate test where both branches go to the same target.
// AFTER (fixed):
if (testBlock->lastIns()->isGoto()) {
testBlock = testBlock->lastIns()->toGoto()->target();
}
// Explicitly checks the instruction type, not the successor count.The vulnerability is in how FoldTests handles CFG shapes that violate its assumptions in release builds.
The key insight: FoldTests pattern matching uses getSuccessor(0) and numSuccessors() == 1 to navigate the CFG, and the safety checks (MOZ_ASSERT) that validate these assumptions are compiled out in release builds. This means:
-
A JavaScript function that creates a CFG where a block's successor list doesn't match FoldTests' expectations will cause incorrect folding.
-
The most likely trigger: A function with a conditional test chain where one branch has been modified by a prior optimization pass (e.g., a pass that adds or removes edges), leaving the block with a successor list that doesn't match what FoldTests expects. Specifically:
- FoldTests calls
trueBranch->getSuccessor(0)expecting it to be the phi block - But a prior pass has changed the successor ordering or the block now ends in an MTest instead of MGoto
- FoldTests reads the wrong successor, redirects edges incorrectly
- The result: a test instruction's true/false branches point to the wrong blocks
- This means a bounds check (which is a test:
if (index < length) goto inBounds else goto oob) can be folded incorrectly so that the "in bounds" path is taken even when the index is out of bounds
- FoldTests calls
-
Concrete trigger pattern: We need JavaScript that creates a pattern like:
function trigger(arr, cond1, cond2, idx) {
// Creates a diamond or triangle CFG pattern with nested tests
// that FoldTests will try to optimize
let val;
if (cond1) {
val = true;
} else {
val = false;
}
// Second test on the phi of val
// FoldTests will try to fold this into the first test
if (val) {
// Access that should be bounds-checked
return arr[idx];
}
return 0;
}But the actual trigger needs to create a specific CFG shape where:
- A prior optimization pass (likely GVN or DCE) modifies the block structure
- FoldTests then encounters a block where
getSuccessor(0)returns the wrong target - The folded test directs control flow to the wrong block
- A bounds check is effectively bypassed
- The
DebugOnly<size_t>→size_tchange is also significant: The variableoldNumPredwas only available in debug builds. In release builds, the progress checkphiBlock->numPredecessors() + 1 == oldNumPredwas not only unchecked (due toMOZ_ASSERT), butoldNumPreditself was optimized out. This means the FoldTests loop that removes predecessors one by one had no progress guarantee in release builds - it could infinite-loop or corrupt the predecessor list.
The FoldTests optimization pass in SpiderMonkey's JIT compiler had multiple assumptions about CFG structure that were validated only in debug builds (MOZ_ASSERT). In release builds (what all users run), these assertions were compiled out. A specific CFG pattern - likely involving nested conditionals where a prior optimization pass modifies successor ordering - causes FoldTests to:
- Navigate to the wrong successor via
getSuccessor(0)when the block doesn't end in a goto - Fold a test incorrectly, redirecting its true/false edges to wrong targets
- Bypass a bounds check if the folded test was guarding an array access
This gives an attacker out-of-bounds TypedArray access from pure JavaScript, with no user interaction.
FoldTests runs BEFORE GVN in the Ion pipeline (Ion.cpp):
- FoldTests (line 1014)
- SplitCriticalEdges
- RenumberBlocks
- AliasAnalysis
- GVN (line ~1210)
This means degenerate MTest blocks created by GVN cannot trigger the bug - they don't exist yet when FoldTests runs. The exploitable CFG shape must come from the Warp frontend (WarpBuilder/WarpTranspiler) or passes that run before FoldTests.
Running trigger candidates in Firefox 148 debug build confirms:
- Functions DO get Warp-compiled (confirmed via
[IonScripts] Warp Compiling) - GVN DOES fold MTest→MGoto (confirmed:
Folded control instruction Test16 to Goto42) - BoundsCheck instructions ARE generated
- But GVN folding happens AFTER FoldTests, so it can't create the pre-condition
The trigger must create a CFG shape where testBlock->numSuccessors()==1 but testBlock->lastIns() is NOT MGoto - and this shape must exist BEFORE FoldTests runs (i.e., it must come from Warp).
Possible Warp-generated shapes to investigate:
- WarpCacheIR stubs that create blocks with degenerate successor lists
- Inlined function returns where both paths merge to the same continuation
- Try/catch blocks where exception and normal paths converge
- Nested ternaries that Warp doesn't simplify
The exact trigger likely requires deeper analysis of how WarpBuilder generates MIR blocks from specific bytecode patterns - this is where Ghidra/Capstone analysis of the WarpBuilder::build_* methods may be needed.
MTest inherits from MAryControlInstruction<1, 2> - the Successors template parameter is 2 and numSuccessors() always returns 2. The numSuccessors()==1 check at lines 825/1011/1197 can ONLY match MGoto blocks. The degenerate-MTest theory is ruled out.
This means the bug is NOT about numSuccessors()==1 matching an MTest. It's about one or more of these validated-by-debug-assert-only invariants being violated:
phi->numOperands() == 2(line 847) - could the phi have 3+ operands?test->ifFalse() == phiBlock(line 1287) - could ifFalse point elsewhere?!pred->isLoopBackedge()(line 1259) - could a predecessor be a backedge?phiBlock->numPredecessors() + 1 == oldNumPred(line 1298) - could the loop not make progress?
All of these are MOZ_ASSERT (debug-only) in the pre-patch code. Any of them being false in a release build would cause silent corruption.
- 14 manual trigger candidates tested - all exercise FoldTests but none violate the invariants
- 400+ fuzzer-generated tests - all engage FoldTests (100% match rate) but none produce the specific violation
- The fuzzer grammar generates ternary/AND/OR/nullish patterns - these produce clean diamonds/triangles
- The trigger likely requires a pattern outside our grammar: possibly involving OSR, loop edge splitting, or a specific interaction between PruneUnusedBranches/FoldEmptyBlocks and FoldTests that creates an invalid invariant
This is the most significant finding from the RE session.
Disassembly of the MaybeFoldTestBlock while loop in the release binary (at 0xbfa934) reveals:
0xbfa93c: ldr x8, [x21, #0x40] // phiBlock->getPredecessor(0)
0xbfa944: ldr x1, [x8] // pred
0xbfa948: ldr x8, [x1, #0x30] // pred->lastIns()
0xbfa95c: ldr x2, [x8, #0x78] // pred->lastIns() + 0x78 = "input()"
0xbfa958: ldr x3, [x8, #0x88] // pred->lastIns() + 0x88 = "ifTrue()"
The code reads offsets 0x78 and 0x88 from the last instruction, assuming it is MTest. There is no opcode check in the release binary - the pred->lastIns()->toTest() call compiles away to a raw pointer cast.
For MTest (expected): MAryControlInstruction<1, 2>
- Offset 0x78 =
operands_[0](MUse → condition input) - Offset 0x88 =
successors_[0](ifTrue block) - Offset 0x90 =
successors_[1](ifFalse block)
For MGoto (type confusion): MAryControlInstruction<0, 1>
- Offset 0x78 =
successors_[0](goto target) - read as "input" - Offset 0x88 = past end of object - heap data - read as "ifTrue"
- Offset 0x90 = past end of object - heap data - read as "ifFalse"
If a predecessor ending in MGoto enters the while loop, the code reads heap garbage as block pointers and uses them to rewire control flow. This is an exploitable heap OOB read → CFG corruption.
How to reach this path: The validation loop (lines 1242-1257) catches non-test predecessors and bails. But it relies on MOZ_ASSERT for the progress guarantee (line 1298). If UpdateTestSuccessors processes a predecessor but doesn't remove it from the predecessor list, the while loop re-processes the same predecessor - but now its test has been updated to point AWAY from phiBlock. The second iteration's test->ifTrue() == phiBlock check fails, AND test->ifFalse() == phiBlock check (the debug-only assertion) isn't checked in release. The code proceeds with wrong assumptions.
The DebugOnly<size_t> removal is the key. Without the progress check:
- First iteration: processes pred correctly, updates its test targets
- But numPredecessors doesn't decrement (edge case in UpdateTestSuccessors)
- Second iteration: same pred, but its test now points elsewhere
- Code reads
test->ifTrue()- it's NOT phiBlock anymore - Falls to the else branch (line 1287):
MOZ_ASSERT(test->ifFalse() == phiBlock)- compiled out in release - Calls
UpdateTestSuccessorswith wrong parameters → CFG corruption
The RE engineer should focus on:
- Which callers invoke
SplitCriticalEdgesForBlockbefore FoldTests runs (line 846) - Whether
FoldEmptyBlockscan create a phi with mismatched predecessor count - Whether
PruneUnusedBranchescan leave a phi's predecessor as a loop backedge - Using a JS fuzzer (jsfunfuzz) with custom grammar rules for FoldTests patterns
The trigger function needs to:
- Create a diamond or triangle pattern with nested tests
- Include enough complexity that a prior optimization pass (GVN, DCE, or similar) modifies the CFG before FoldTests runs
- Cause FoldTests to encounter a block where
getSuccessor(0)≠ goto target - Have a TypedArray bounds check in the path that gets incorrectly folded
The DebugOnly change suggests the multi-predecessor FoldTests variant (the loop at the bottom of the patch) is also a trigger candidate - if we can get that loop to corrupt the predecessor list.
Recommended approach: Build debug Firefox 148, add logging to the FoldTests pass, then progressively construct JavaScript that exercises the diamond/triangle patterns until we hit the miscompilation in release builds.
| Theory | Status | Reason |
|---|---|---|
| Degenerate MTest (same true/false target) | Ruled out | MTest's numSuccessors() always returns 2 (template parameter). numSuccessors()==1 only matches MGoto. |
| GVN creates the condition | Ruled out | GVN runs AFTER FoldTests in the pass pipeline. |
| WarpBuilder creates degenerate MTest | Ruled out | WarpBuilder has explicit target1 == target2 → buildForwardGoto dedup (line 1416-1419). |
| Standard JS patterns (ternary, AND, OR, ??, ?., try/catch, generators, for-of, switch, inlined functions) | Ruled out | All produce clean Goto blocks in the testBlock position. |
The patch upgrades 4 MOZ_ASSERT to MOZ_RELEASE_ASSERT. Each represents an invariant that CAN be violated in release builds. Violating any one is sufficient for a trigger.
Invariant 1: phi->numOperands() == 2 (line 847)
- FoldTests diamond handler assumes the phi has exactly 2 operands
- If a phi has 3+ operands (from edge splitting, OSR entry, or prior pass restructuring),
indexForPredecessor(trueBranch)andindexForPredecessor(falseBranch)may return wrong indices or out-of-bounds
Invariant 2: test->ifFalse() == phiBlock (line 1287)
- The while loop
pred->lastIns()->toTest()assumes every remaining predecessor ends in MTest - If a predecessor ends in MGoto,
toTest()is undefined behavior in release builds
Invariant 3: !pred->isLoopBackedge() (line 1259)
- FoldTests assumes no predecessor of the phiBlock is a loop backedge
- If a predecessor IS a backedge, the transformation corrupts loop structure
Invariant 4: phiBlock->numPredecessors() + 1 == oldNumPred (line 1298)
- The while loop assumes it removes exactly one predecessor per iteration
oldNumPredisDebugOnly<size_t>- it doesn't exist in release builds, so this check never runs- If
UpdateTestSuccessorsfails to remove a predecessor, the loop processes the same predecessor repeatedly