CVE-2025-0291
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
forsrc/compiler/turboshaft/wasm-gc-typed-optimization-reducer.cc |
modified | |
ifsrc/compiler/turboshaft/wasm-gc-typed-optimization-reducer.cc |
modified |
Files Changed
src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.ccsrc/compiler/turboshaft/wasm-gc-typed-optimization-reducer.h
Patch
From f231d83cb3c08754413b3ee1aa249cebd4d5445f Mon Sep 17 00:00:00 2001 From: Matthias Liedtke <[email protected]> Date: Thu, 12 Dec 2024 14:11:00 +0100 Subject: [PATCH] [turboshaft][wasm] WasmGCTypeAnalyzer: Fix phi input for single-block loops Fixed: 383356864 Change-Id: Idc644923c2e09e16b0c4c1cb1cda8f5c3d8189d9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6087921 Reviewed-by: Jakob Kummerow <[email protected]> Reviewed-by: Nico Hartmann <[email protected]> Commit-Queue: Matthias Liedtke <[email protected]> Cr-Commit-Position: refs/heads/main@{#97723} --- diff --git a/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.cc b/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.cc index a051d9b..15a70a3 100644 --- a/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.cc +++ b/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.cc @@ -307,6 +307,28 @@ wasm::ValueType::Ref(type_index), allocate_struct); } +wasm::ValueType WasmGCTypeAnalyzer::GetTypeForPhiInput(const PhiOp& phi, + int input_index) { + OpIndex phi_id = graph_.Index(phi); + OpIndex input = ResolveAliases(phi.input(input_index)); + // If the input of the phi is in the same block as the phi and appears + // before the phi, don't use the predecessor value. + + if (current_block_->begin().id() <= input.id() && input.id() < phi_id.id()) { + // Phi instructions have to be at the beginning of the block, so this can + // only happen for inputs that are also phis. Furthermore, this is only + // possible in loop headers of loops with a single block (endless loops) and + // only for the backedge-input. + DCHECK(graph_.Get(input).Is<PhiOp>()); + DCHECK(current_block_->IsLoop()); + DCHECK(current_block_->HasBackedge(graph_)); + DCHECK_EQ(current_block_->LastPredecessor(), current_block_); + DCHECK_EQ(input_index, 1); + return types_table_.Get(input); + } + return types_table_.GetPredecessorValue(input, input_index); +} + void WasmGCTypeAnalyzer::ProcessPhi(const PhiOp& phi) { // The result type of a phi is the union of all its input types. // If any of the inputs is the default value ValueType(), there isn't any type @@ -320,12 +342,10 @@ phi); return; } - wasm::ValueType union_type = - types_table_.GetPredecessorValue(ResolveAliases(phi.input(0)), 0); + wasm::ValueType union_type = GetTypeForPhiInput(phi, 0); if (union_type == wasm::ValueType()) return; for (int i = 1; i < phi.input_count; ++i) { - wasm::ValueType input_type = - types_table_.GetPredecessorValue(ResolveAliases(phi.input(i)), i); + wasm::ValueType input_type = GetTypeForPhiInput(phi, i); if (input_type == wasm::ValueType()) return; // <bottom> types have to be skipped as an unreachable predecessor doesn't // change our type knowledge. @@ -343,8 +363,7 @@ if (v8_flags.trace_wasm_typer) { for (int i = 0; i < phi.input_count; ++i) { OpIndex input = phi.input(i); - OpIndex underlying = ResolveAliases(input); - wasm::ValueType type = types_table_.GetPredecessorValue(underlying, i); + wasm::ValueType type = GetTypeForPhiInput(phi, i); TRACE("- phi input %d: #%u(%s) -> %s\n", i, input.id(), OpcodeName(graph_.Get(input).opcode), type.name().c_str()); } diff --git a/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.h b/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.h index f8bf6b7..048d508 100644 --- a/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.h +++ b/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.h @@ -86,6 +86,8 @@ void ProcessPhi(const PhiOp& phi); void ProcessTypeAnnotation(const WasmTypeAnnotationOp& type_annotation); + wasm::ValueType GetTypeForPhiInput(const PhiOp& phi, int input_index); + void CreateMergeSnapshot(const Block& block); bool CreateMergeSnapshot(base::Vector<const Snapshot> predecessors, base::Vector<const bool> reachable);
Original Bug Report
WasmGCTypeAnalyzer improperly revisits single-block loops, leading to type confusion
WasmGCTypeAnalyzer is a Turboshaft analyzer responsible for inferring known type information for various operations, which can then be used by WasmGCTypedOptimizationReducer to potentially remove some type checks at compile time. To do this, it traverses the Turboshaft graph while keeping track of which types might be encountered at any particular point in the code. When encountering a loop, the analyzer keeps revisiting the loop body indefinitely until the type feedback stabilizes - this is intended to ensure that the loop backedge’s type feedback is properly accounted for. The analyzer revisits the loop’s body by calling iterator.MarkLoopForRevisitSkipHeader() - the loop header has already been revisited while trying to determine whether the type feedback stabilized, so it is skipped here, presumably as an optimization.
However, this assumption / optimization doesn’t account for single-block loops, which can occur in the Turboshaft graph when compiling infinite loops without any branches. For these loops, the loop header contains the entire loop body, effectively turning iterator.MarkLoopForRevisitSkipHeader() into a No-Op, and preventing the loop from being revisited any additional times. This cuts the fixed-point analysis short after two iterations: one initial visit, and one additional visit as part of the check to determine whether the type feedback stabilized. As such, WasmGCTypeAnalyzer might report incorrect type feedback to WasmGCTypedOptimizationReducer when the loop’s type information does not stabilize after two iterations (which can be the case because of, for example, a chain of Phis), which can lead to type checks being incorrectly removed, resulting in type confusion.
See attached two files:
poc.js: this contains a minimal POC which utilizes this bug to create and dereference a fake object, resulting in a crash. This POC has been reproduced with an D8 optdebug build (commit86ffa19a533bdec3b9ce4a56106a2d9e8015e108) on an x86-64 Linux machine.exploit.js: this contains a full Chromium exploit chain, utilizing this bug + unprotected PartitionAlloc metadata to demonstrate an attacker controlled write outside of the sandbox. This exploit has been tested and confirmed to work on Chromium 131.0.6778.108 on an x86-64 Linux machine, using--js-flags="--turboshaft-wasm"to enable Turboshaft for Wasm. Note that this class of sandbox escape (namely manipulating exposedSlotSpanMetadataobjects) has been used in other exploit chains to achieve full RCE; this exploit stops short of achieving RCE, only demonstrating attacker controlled writes, since further exploitation requires hardcoding offsets for specific Chromium builds, which reduces the general reproducibility and reliability of the exploit.
A potential fix for this bug would be to not use MarkLoopForRevisitSkipHeader for revisiting single block loops, and instead use MarkLoopForRevisit for this particular edge case. A sample patch of such a fix could look as follows:
@@ -61,8 +61,14 @@ void WasmGCTypeAnalyzer::Run() {
if (needs_revisit) {
block_to_snapshot_[loop_header.index()] = MaybeSnapshot(snapshot);
// This will push the successors of the loop header to the iterator
- // stack, so the loop body will be visited in the next iteration.
- iterator.MarkLoopForRevisitSkipHeader();
+ // stack, so the loop body will be visited in the next iteration. If
+ // this is a single-block loop, then there are no successors - as such
+ // revisit the entire loop (consisting of just the header) in this
+ // case.
+ if (block.index() != header.index()) {
+ iterator.MarkLoopForRevisitSkipHeader();
+ } else {
+ iterator.MarkLoopForRevisit();
+ }
}
}
}
Reporter Credit: if applicable, please credit my pseudonym Popax21 in regards to this report.