4e81b0721cc375c8e629ae50bf23b9f36c9c32ac  Fix unsafe buffer usage in BackgroundFetchRegistration

The bug at a glance

This is a spanification/unsafe-buffer cleanup, not a fix for a demonstrated memory-corruption bug. The change was auto-generated by the headless gemini-cli spanification agent (//agents/prompts/projects/spanification/run.py) and its stated purpose is to remove UNSAFE_TODO pointer arithmetic from an erase-while-iterating loop. The commit carries CVSS 0.0 and exploitable=false; it is tagged isSecurity=true only because it eliminates a latent-safety pattern. There is no evidence the original loop was actually reachable in a way that produced an invalid iterator, and HeapVector::erase returns a valid next iterator, so the pre-existing code was very likely correct in practice. This is honest watch-tier: a defensive rewrite that removes an UNSAFE_TODO marker and a fragile idiom. Low severity, no CVSS assigned by us (null) because there is no substantiated exploitable defect.

Treat this as codebase hygiene, not a vulnerability you must chase. BackgroundFetchRegistration kept a list of BackgroundFetchRecord observers and, on each completed request, walked the vector erasing matching entries in place using an UNSAFE_TODO(it++) increment on the non-erased branch. The Chrome unsafe-buffers program flags manual iterator arithmetic like this because it is the shape that, when combined with re-entrancy or a bad erase, can produce out-of-bounds access. The rewrite sidesteps the whole class by building a fresh remaining-observers vector and swapping it in. Defender value: know that this code path was audited and made robust; there is no shipped exploit and no proof the old code ever misbehaved.

Root cause

OnRequestCompleted iterated observers_ (a HeapVector<Member>). For each observer whose ObservedUrl matched the completed request’s url, it called observer->OnRequestCompleted(response) and erased the element, reassigning the iterator from erase(). For non-matching observers it advanced with UNSAFE_TODO(it++). Two things make this pattern fragile enough for the unsafe-buffers program to rewrite it: the manual it++ is raw iterator arithmetic the spanification tooling wants gone, and calling into observer->OnRequestCompleted() mid-iteration is a re-entrancy hazard — if that callback could mutate observers_ (add/remove records), the live iterator over the same vector could be invalidated, giving an out-of-bounds or use-after-free-style read on the backing store.

The fix removes the in-place erase-and-increment dance entirely. It builds a new local HeapVector remaining_observers, iterates observers_ by reference, invokes OnRequestCompleted on matches (without erasing during the walk), pushes non-matches into remaining_observers, and finally observers_.swap(remaining_observers). No iterator is advanced by hand and no element is erased while iterating, so the UNSAFE_TODO annotation is deleted. The behavior is subtly safer against re-entrancy too: mutations a callback makes to observers_ during the loop no longer corrupt the iteration, because the loop reads the original vector and only the swap at the end installs the result. This is a latent-safety hardening; there is no evidence a crash or corruption was observed in the wild.

Key code

Before: erase-while-iterating with UNSAFE_TODO pointer/iterator arithmetic. After: build a remaining-observers vector and swap.

-  for (auto it = observers_.begin(); it != observers_.end();) {
-    BackgroundFetchRecord* observer = it->Get();
+  HeapVector<Member<BackgroundFetchRecord>> remaining_observers;
+  for (auto& observer : observers_) {
     if (observer->ObservedUrl() == request->url) {
       observer->OnRequestCompleted(response->Clone());
-      it = observers_.erase(it);
     } else {
-      UNSAFE_TODO(it++);
+      remaining_observers.push_back(observer);
     }
   }
+  observers_.swap(remaining_observers);

Patch walkthrough

• third_party/blink/renderer/modules/background_fetch/background_fetch_registration.cc — OnRequestCompleted is rewritten. The manual begin/end loop with in-place erase and UNSAFE_TODO(it++) is replaced by a range-for over observers_ that dispatches OnRequestCompleted to matching observers, accumulates non-matching observers into a new HeapVector remaining_observers, and swaps that into observers_ at the end. Same net effect (matched observers notified and removed) with no hand-written iterator arithmetic and no mutation of the container during iteration.

Background

UNSAFE_TODO — A Chrome macro marking a spot that still uses unsafe pointer/iterator arithmetic and is queued for the unsafe-buffers (spanification) cleanup. Its presence is a to-do, not proof of a live bug; the program’s goal is to eliminate the pattern wholesale.

Spanification agent — The commit body notes the initial patchset was generated by headless gemini-cli via //agents/prompts/projects/spanification/run.py — an automated pass that rewrites unsafe buffer/iterator code, then human-reviewed (Sonzogni, Beverloo).

erase-while-iterating hazard — Erasing from a container during iteration and manually advancing the iterator is a classic invalidation trap, especially when the loop body re-enters code that can mutate the same container. The safe idioms are erase-remove, or building a new container and swapping, as done here.

BackgroundFetchRecord observers — observers_ tracks per-request record objects that want completion notifications for a Background Fetch. OnRequestCompleted fans out the response to records watching the completed URL.

Vulnerability window

1. Origin — Merged 2026-01-21 (Andy Phan), refs/heads/main@{#1572647}, Fixed: 457304341 (a spanification tracking bug).
2. Distribution — No [M###] backport tag present; treated as mainline-only hardening at time of review.
3. Analyst review — 2026-09-27: classified watch-tier low, CVSS null; hardening with latent-safety motive, no demonstrated exploit.

Proof of concept

No test ships in this diff and no exploit is demonstrated. The only theoretical trigger would be arranging for BackgroundFetchRecord::OnRequestCompleted to re-enter and mutate observers_ mid-loop, which could invalidate the live iterator on a pre-patch build. Whether that re-entrancy is reachable from web content is not established here, and the pre-patch erase() already returned a valid iterator, so ordinary operation was almost certainly safe. No PoC is claimed and none should be fabricated.

Exploitation

1. Reachability — Requires a Background Fetch with active record observers and a completing request. The corruption path additionally requires OnRequestCompleted to mutate observers_ during iteration; that re-entrancy is unproven, which is the main thing blocking any exploitation story.
2. Primitive — At most an invalidated-iterator read over a HeapVector’s backing store. HeapVector is Oilpan-managed and the erase() idiom returned valid iterators, so even the invalidation case is closer to a logic/crash bug than a controllable corruption primitive.
3. Ceiling — No memory-write primitive is demonstrated. Realistic worst case is a renderer crash under a contrived re-entrancy; there is no path to escalation shown, keeping this watch-tier.

Detection & hunting

For defenders and detection engineers:

• Crash frames —
• No network IOC —

Audit directions

• Remaining UNSAFE_TODO iterator loops —
• Re-entrancy into observer lists —