0afb939d222cf7d03862245c7920767e2777a530 Avoid stale iterators in SVGResource cycle checks
Triage note: Fix moves clients_.find(&client) to after ResourceContainerForCycleCheck() because that call can run a lifecycle update that allocates and triggers GC, invalidating the iterator whose it->value is dereferenced. Replaces held iterator with Contains() pre-check + re-find. SVG resource cycle is attacker-reachable.
The bug at a glance
Use of a GC-invalidated HeapHashMap iterator in Blink’s SVGResource, a renderer-process memory-safety bug reachable from attacker-controlled SVG (resource references forming a potential cycle). Dereferencing a stale iterator (it->value) after Oilpan GC moved/freed backing store is a UAF-class read/write. High, though exploitability depends on whether the lifecycle update triggers a GC that moves clients_ in the window.
SVGResource::ResourceContainer() and FindCycle() looked up a clients_ iterator, then called ResourceContainerForCycleCheck() which can run a lifecycle update that allocates and triggers an Oilpan GC. Oilpan can move/rehash the HeapHashMap backing store, so the previously obtained iterator (and it->value) is stale; using it after the call is a use of invalidated heap memory.
Root cause
Both functions originally did auto it = clients_.find(&client) and then called ResourceContainerForCycleCheck() while still holding it, dereferencing it->value.cached_cycle_check afterward. ResourceContainerForCycleCheck() can perform a lifecycle update that allocates memory and triggers a garbage collection, which invalidates iterators into the clients_ HeapHashMap. The fix reorders the operations: it first tests membership with clients_.Contains(&client) (no retained iterator), calls ResourceContainerForCycleCheck(), and only after the GC-capable call re-acquires the iterator via clients_.find(&client), guarded by CHECK(it != clients_.end()). Because the iterator is obtained after the last GC-triggering point, it->value is read from valid, post-GC backing store.
Key code
Test membership first, run the GC-capable call, then re-acquire the iterator.
if (!clients_.Contains(&client)) {
return nullptr;
}
auto* container = ResourceContainerForCycleCheck();
if (!container) {
return nullptr;
}
// ResourceContainerForCycleCheck() can perform a lifecycle update which
// allocates memory and triggers a GC that invalidates iterators.
auto it = clients_.find(&client);
CHECK(it != clients_.end());
if (it->value.cached_cycle_check == kNeedCheck) {
Patch walkthrough
third_party/blink/renderer/core/svg/svg_resource.cc— In both SVGResource::ResourceContainer() and SVGResource::FindCycle(), replaces the early auto it = clients_.find(&client) with a clients_.Contains(&client) membership test that retains no iterator. The GC-capable ResourceContainerForCycleCheck() call is made next, and only afterward is the iterator re-acquired with clients_.find(&client) and validated by CHECK(it != clients_.end()). A comment records that ResourceContainerForCycleCheck() can allocate and trigger a GC that invalidates iterators.
Background
SVGResource / clients_ — SVGResource tracks its SVGResourceClients in a HeapHashMap (clients_) mapping client -> per-client state including cached_cycle_check. Entries and the backing store are Oilpan-managed.
HeapHashMap iterator invalidation — Oilpan can move or rehash heap-allocated containers during GC, invalidating any live iterator into a HeapHashMap. Holding an iterator across an allocation/GC point is a use-after-free hazard.
ResourceContainerForCycleCheck() — Returns the LayoutSVGResourceContainer for the cycle check; it can perform a lifecycle update that allocates memory and may trigger a GC — the event that invalidates the retained iterator.
cached_cycle_check (kNeedCheck/kPerformingCheck) — Per-client cached state stored in it->value used to memoize and guard SVG resource cycle detection. Reading it through a stale iterator is the sink.
Vulnerability window
- Setup — A document contains SVG with resource references; a client is registered in SVGResource::clients_, and layout invokes ResourceContainer()/FindCycle() for cycle checking.
- Stale ref — The function obtains auto it = clients_.find(&client) and holds it.
- GC — ResourceContainerForCycleCheck() performs a lifecycle update that allocates and triggers an Oilpan GC, moving/rehashing the clients_ backing store and invalidating it.
- UAF — The code dereferences it->value.cached_cycle_check on the now-invalid iterator, reading/writing freed or relocated heap memory.
- Fixed — Iterator is re-acquired after the GC-capable call and CHECK-validated, so it->value is always read from valid post-GC memory.
Proof of concept
No test or PoC is included; it is a source-only fix with explanatory comments. Conceptual trigger grounded in the patch: craft SVG whose resource references form (or nearly form) a cycle so layout invokes SVGResource::ResourceContainer()/FindCycle() for a registered client, and arrange for ResourceContainerForCycleCheck()’s lifecycle update to allocate enough to trigger an Oilpan GC that moves/rehashes the clients_ HeapHashMap during the call; pre-fix the retained iterator is then dereferenced stale. Deterministically forcing the GC to move clients_ in that window (and grooming to make the stale read exploitable) is (inferred).
Exploitation
- Reach — Load attacker SVG whose resource references drive SVGResource cycle checking (ResourceContainer()/FindCycle()) for a registered client during the lifecycle update.
- Force GC — (inferred) Shape allocations so ResourceContainerForCycleCheck()’s lifecycle update triggers an Oilpan GC that moves/rehashes the clients_ HeapHashMap while the stale iterator is held.
- Stale access — The pre-fix dereference of it->value.cached_cycle_check reads (and, on write-back of kPerformingCheck, writes) relocated/freed memory.
- Escalate (inferred) — Groom the Oilpan heap so the moved/freed slot holds controlled data, turning the stale read/write into a corruption primitive; no sink or escalation is shown by the patch.
Detection & hunting
For defenders and detection engineers:
- Iterator held across a GC-capable call — Flag any HeapHashMap/HeapHashSet iterator that is live across a call which can allocate or run a lifecycle update / GC. Re-acquire the iterator after such calls.
- it->value dereference after lifecycle/layout call — In Blink SVG/layout code, audit reads of it->value following ResourceContainerForCycleCheck() or any UpdateLifecycle/layout call for stale-iterator use.
- Oilpan heap verifier / ASAN — Run with Blink GC stress / conservative-GC-at-every-allocation and ASAN to surface stale HeapHashMap iterator dereferences under SVG cycle-check workloads.
- Missing CHECK after re-find — Where membership is assumed to persist across a GC, require Contains() + post-call find() + CHECK(it != end()).
Audit directions
- SVGResource and SVG resource containers — Review all SVGResource methods and LayoutSVGResourceContainer cycle/lifecycle code for other iterators or raw pointers into clients_ retained across ResourceContainerForCycleCheck() or lifecycle updates.
- Blink HeapHashMap/HeapHashSet iterator lifetimes — Sweep core/svg and core/layout for find()->value kept across allocation/GC points; convert to Contains()+re-find() or snapshot the value before the GC-capable call.
- Lifecycle-update reentrancy — Enumerate call sites that invoke a lifecycle/layout update while iterating or holding references into GC-managed containers.
- cached_cycle_check state machine — Verify kNeedCheck/kPerformingCheck transitions always operate on a freshly-acquired, validated iterator.