e886fddb58ee841deaea07706ac1264de7900992  Guard against re-hashing in SVGResource::ResourceContainer

The bug at a glance

The dangling reference lives in SVGResource::ResourceContainer(), the hot path that resolves an element’s referenced SVG resource (filter/clip/mask/marker/paint-server) and runs a cycle check. SVG resource references and their cycle checks are fully web-reachable from untrusted markup/CSS, so an attacker controls both the clients_ map contents and the reentrancy that mutates it during FindCycle(). A stale ClientEntry& that is written through after the backing HashMap rehashes is a classic use-after-free/out-of-bounds write of attacker-influenced map storage inside the renderer, i.e. a renderer-process memory-corruption primitive reachable without special privilege. That combination (web-reachable, write-through-stale-ref, renderer heap) is the profile of a High-severity Blink UAF.

Everyone audits the obvious ‘iterator invalidated by erase’ pattern. The subtle bug here is that a const/non-const reference bound to a HashMap value (ClientEntry& entry = it->value) is silently invalidated not by an erase on this map, but by a reentrant call – container->FindCycle() – that walks the resource graph and ends up inserting new clients into the very same clients_ map, triggering a rehash/reallocation of its backing store. The reference ’entry’ still points at the old, freed bucket, and the code then writes entry.cached_cycle_check through it. The angle for analysts: reference lifetime across a callback is as dangerous as iterator lifetime, and ‘I only read/wrote one field’ does not save you when the whole table moved.

Root cause

In ResourceContainer(), the pre-patch code binds ClientEntry& entry = it->value into the clients_ hash map, then enters the kNeedCheck branch and calls container->FindCycle(). FindCycle() walks the SVG resource graph and can re-enter resource resolution for other elements, which mutates clients_ (inserting entries) and rehashes it, reallocating the bucket array so that entry (and it) now dangle at freed storage. The original code then writes entry.cached_cycle_check = has_cycle ? kHasCycle : kNoCycle through that stale reference and later reads entry.cached_cycle_check, a use-after-free write/read on the old table. The fix drops the cached reference across the callback: after FindCycle() returns it re-does it = clients_.find(&client) and asserts CHECK(it != clients_.end()), so every subsequent access (it->value.cached_cycle_check) goes through the freshly located, valid bucket. Because the lookup is redone against the current table, the rehash is harmless, and the CHECK converts a ‘client somehow gone’ state into a safe controlled abort instead of another dangling access.

Key code

svg_resource.cc: re-find the client entry after FindCycle() can rehash clients_, replacing the stale ClientEntry& with a revalidated iterator guarded by CHECK.

if (it->value.cached_cycle_check == kNeedCheck) {
  it->value.cached_cycle_check = kPerformingCheck;
  bool has_cycle = container->FindCycle();
  // FindCycle() can modify clients_ and invalidate the iterator
  it = clients_.find(&client);
  CHECK(it != clients_.end());
  DCHECK_EQ(it->value.cached_cycle_check, kPerformingCheck);
  it->value.cached_cycle_check = has_cycle ? kHasCycle : kNoCycle;
}
if (it->value.cached_cycle_check == kHasCycle) {
  return nullptr;
}
DCHECK_EQ(it->value.cached_cycle_check, kNoCycle);
return container;

Patch walkthrough

• third_party/blink/renderer/core/svg/svg_resource.cc — Removes the long-lived binding ClientEntry& entry = it->value;. That reference was captured before the reentrant FindCycle() call and dangled once clients_ rehashed; every later entry.* access was a use-after-free.
• third_party/blink/renderer/core/svg/svg_resource.cc — Inside the kNeedCheck branch, after bool has_cycle = container->FindCycle();, adds it = clients_.find(&client); CHECK(it != clients_.end()); with the comment that FindCycle() can modify clients_ and invalidate the iterator. This revalidates the iterator against the current (possibly reallocated) table before the write-back.
• third_party/blink/renderer/core/svg/svg_resource.cc — Rewrites the write-back and the two trailing reads to go through it->value.cached_cycle_check instead of the stale entry.cached_cycle_check (the kPerformingCheck->kHasCycle/kNoCycle transition, the kHasCycle early-return, and the final DCHECK_EQ(…, kNoCycle)), so all post-FindCycle accesses use the revalidated iterator.

Background

SVGResource::ResourceContainer() — Blink method that resolves the LocalSVGResource’s container element and, on first use per client, runs a cycle check so an SVG resource reference graph (filters, clips, masks, markers, paint servers) cannot recurse infinitely. It is the function patched here.

clients_ (HashMap of client -> ClientEntry) — Per-resource map tracking each referencing client and its cached cycle-check state. Like all Blink HashMaps, inserting entries can rehash and reallocate the backing store, invalidating any iterator or reference (ClientEntry&) bound to a value in it.

FindCycle() — Called on the resolved container to detect reference cycles in the SVG resource graph. It walks to other resources/clients and can re-enter resolution, causing insertions into clients_ – the reentrancy that rehashes the map mid-function.

cached_cycle_check state machine — ClientEntry.cached_cycle_check moves kNeedCheck -> kPerformingCheck (set before FindCycle) -> kHasCycle/kNoCycle (set after). kPerformingCheck guards against reentrant re-evaluation for the same client; the bug was that the post-FindCycle state write landed through a dangling reference.

Vulnerability window

1. Resolve — A layout/paint consumer calls SVGResource::ResourceContainer() for a client; it obtains the container via ResourceContainerNoCycleCheck() and locates the client’s ClientEntry in clients_ (it = clients_.find(&client)).
2. Vulnerable bind — Pre-patch, it binds ClientEntry& entry = it->value and, in the kNeedCheck branch, sets entry.cached_cycle_check = kPerformingCheck.
3. Reentrant mutation — container->FindCycle() walks the resource graph and inserts new entries into clients_, rehashing it and freeing the old bucket array; entry/it now dangle.
4. UAF write/read — Pre-patch, entry.cached_cycle_check is written (kHasCycle/kNoCycle) and read through the stale reference – use-after-free on freed HashMap storage.
5. Fix — Post-patch, the iterator is re-found (clients_.find(&client)) and CHECK’d before any write-back, so all accesses hit valid storage.

Proof of concept

No test is included in the diff. Conceptual trigger grounded in the patch: build an SVG document whose resource references form a graph where resolving one resource’s cycle check re-enters resolution for additional clients, so that FindCycle() causes insertions into the same LocalSVGResource’s clients_ map. (inferred) Construct many referencing clients (e.g. a large set of elements referencing the same resource plus a nested/mutually-referencing resource chain) so the insertion during FindCycle() crosses a rehash threshold and reallocates the bucket array while ResourceContainer() holds the stale ClientEntry&. The dangling write of cached_cycle_check then lands in freed memory. The exact element markup and the load-factor threshold that forces the rehash are (inferred) – the diff only shows the reference being invalidated by FindCycle() mutating clients_, not the DOM shape that reaches it.

Exploitation

1. Reach — Deliver an SVG (inline, , or CSS url()) whose resource references trigger a per-client cycle check through ResourceContainer() on the first resolution – fully web-reachable, no privilege. (inferred) the precise reference topology that makes FindCycle() re-enter and insert into clients_.
2. Force the rehash — (inferred) Arrange enough simultaneous clients of one LocalSVGResource that the insertion performed during FindCycle() pushes clients_ past its load factor, reallocating the backing array exactly while the stale ClientEntry& is held.
3. Primitive — (inferred) The post-FindCycle write of cached_cycle_check (a small enum field) through the dangling reference is a constrained write into the freed former bucket slot; the follow-on reads are UAF reads of the same region.
4. Groom / corrupt — (inferred) Reclaim the freed bucket array with attacker-controlled heap content (PartitionAlloc grooming) so the dangling cached_cycle_check field overlaps a useful object field, converting the stale write into a targeted corruption; chaining to control flow is entirely (inferred) and not shown by the patch.

Detection & hunting

For defenders and detection engineers:

• ASAN heap-use-after-free in SVGResource::ResourceContainer — Fuzzing SVG resource graphs under AddressSanitizer: a UAF read/write whose freed allocation is a Blink HashMap bucket array and whose use site is in ResourceContainer() right after FindCycle() is the exact signature. The freeing stack will run through clients_ insertion/rehash during FindCycle().
• CHECK failure at the new clients_.find(&client) guard — On patched builds, a crash at CHECK(it != clients_.end()) in svg_resource.cc indicates a case where the client vanished from clients_ across FindCycle() – worth investigating as an unexpected reentrancy/removal path rather than dismissing as benign.
• DCHECK_EQ(…, kPerformingCheck) tripping in debug — A debug-build DCHECK failure on the cached_cycle_check state after FindCycle() signals reentrant corruption of the state machine, an early indicator of the same reentrancy class.

Audit directions

• References/iterators bound to Blink HashMap values across callbacks — Grep for patterns like Entry& x = it->value; or a stored it followed by any call that can re-enter and insert into the same map (FindCycle, layout, paint, event dispatch). Re-find or re-index after the callback, or cache by value, not by reference.
• Other FindCycle()/cycle-check consumers in core/svg — Audit all callers of FindCycle() and the SVG resource resolution paths (filters, clip-path, mask, marker, paint servers) for values held across the call that live in collections FindCycle() can mutate.
• Reentrancy into clients_ mutation — Trace every path that inserts into or erases from LocalSVGResource::clients_ and confirm no caller holds a live reference/iterator into clients_ across it; the cached_cycle_check state machine assumes single-entry resolution per client and should be re-examined for other reentrant writes.