119decf26fd735a04f15debc477558988f46697a Use a CheckedObserver list for RenderFrameMetadataProvider
Triage note: Proactive hardening of an untriaged dangling-observer risk; no current live bug identified. Bug:561960454.
The bug at a glance
This is a defense-in-depth type change with no demonstrated reachable use-after-free. The pre-patch observer list was annotated UncheckedAndDanglingUntriaged in 2024 by the automated BackupRefPtr dangling-pointer triage sweep and was never individually reviewed, which means the tooling could not prove the list was safe, not that a live teardown-ordering bug was ever hit. No crash, no test, and no attacker input path is shown in the CL. The realistic worst case is a browser-process UAF only if some Observer implementation is destroyed without unregistering while a notification is in flight; that scenario is not exhibited here. Impact is therefore latent and speculative, scoring low (CVSS ~3.1).
RenderFrameMetadataProviderImpl lives in the browser process and fans out cc::RenderFrameMetadata updates (surface/scroll/viewport metadata driven by compositor frame submissions) to registered observers. The interesting question is whether an observer can outlive its unregistration or be freed mid-dispatch. With an Unchecked list the container stores raw pointers and offers no liveness check, so a stale entry would be dereferenced silently. The patch converts the contract to CheckedObserver, which turns any such teardown mistake into a controlled failure instead of a memory-safety violation. The value here is not a fix for a known reachable bug but the elimination of a whole class of future dangling-observer regressions in a browser-process broadcaster.
Root cause
base::ObserverList::UncheckedAndDanglingUntriaged holds bare T* entries and performs no per-iteration validity check; if an observer is destroyed while still enrolled, the next dispatch dereferences freed memory. base::CheckedObserver instead makes every observer inherit a base::CheckedObserver mixin carrying a WeakPtr-style factory, so ObserverList can detect and skip an entry whose object has been destroyed, and DCHECK/CHECK on incorrect teardown ordering. The diff makes Observer derive from base::CheckedObserver, gives it a virtual override destructor (required so the checked machinery hooks in), and drops the UncheckedAndDanglingUntriaged qualifier from observers_. Functionally the broadcast path is unchanged; the memory-safety guarantee of the container is strengthened from ’trust the caller’ to ‘verify at dispatch’.
Key code
Observer contract switched from unchecked/dangling to CheckedObserver, and the member list loses the Unchecked qualifier.
- class Observer {
+ class Observer : public base::CheckedObserver {
public:
- virtual ~Observer() {}
+ ~Observer() override = default;
// render_frame_metadata_provider_impl.h
- base::ObserverList<Observer>::UncheckedAndDanglingUntriaged observers_;
+ base::ObserverList<Observer> observers_;
Patch walkthrough
content/public/browser/render_frame_metadata_provider.h— Adds #include “base/observer_list_types.h” and makes the public Observer interface derive from base::CheckedObserver. The destructor is changed from an inline empty body to ‘~Observer() override = default;’ — ‘override’ is meaningful now because base::CheckedObserver declares a virtual destructor, and the checked machinery relies on that virtual chain to run its liveness bookkeeping on destruction. Every concrete observer implementing this interface across content/ automatically gains the checked contract.content/browser/renderer_host/render_frame_metadata_provider_impl.h— Changes the member type from base::ObserverList::UncheckedAndDanglingUntriaged to the default base::ObserverList, which for a CheckedObserver element type is a checked list. This is the substantive security change: dispatch through observers_ now validates each entry rather than blindly dereferencing raw pointers, and mis-ordered teardown becomes a diagnosable CHECK rather than a silent UAF.
Background
RenderFrameMetadataProvider — Browser-process object associated with a RenderWidgetHost that surfaces per-frame compositor metadata (scroll offset, page scale, viewport, root background color, selection bounds) to consumers such as input handling and browser controls. It uses the observer pattern to notify multiple subscribers on each frame submission.
UncheckedAndDanglingUntriaged — A base::ObserverList qualifier introduced during the BackupRefPtr (MiraclePtr) rollout. It marks lists that the automated 2024 sweep could not prove safe and that a human never reviewed. It suppresses dangling-pointer diagnostics rather than asserting safety, so it is technical debt, not a certification.
base::CheckedObserver — A mixin that lets base::ObserverList detect when an enrolled observer has been destroyed and skip or CHECK on it. It is the Chromium-preferred safe default for observer lists and converts dangling-observer bugs from exploitable UAFs into fail-fast crashes.
Bug 561960454 / CONV a5de1ab9 — The commit carries TAG=agy and a CONV id, consistent with an agent/automation-assisted cleanup pass over remaining Unchecked observer lists rather than a report of a live exploit.
Vulnerability window
- 2024 annotation — BRP triage sweep tags observers_ as UncheckedAndDanglingUntriaged without individual review.
- 2026-09-16 fix — Jonathan Ross converts the Observer contract to base::CheckedObserver; reviewed by Arthur Sonzogni, landed at main@{#1699029}.
- post-landing — No known in-the-wild trigger; classified as hardening/watch.
Proof of concept
No exploit or test ships with this CL, and none is warranted because no reachable UAF is demonstrated. A conceptual repro would require finding a concrete RenderFrameMetadataProvider::Observer subclass that can be destroyed while still registered and while a metadata notification is being dispatched — i.e. a teardown-ordering race in a consumer. Even that would only manifest with the pre-patch Unchecked list; the whole point of the change is that no such path was ever proven, so no exploit is demonstrated.
Exploitation
- precondition — Would require a consumer that unregisters late or is freed mid-broadcast; not shown to exist.
- primitive — If it existed, a freed Observer* dereference during dispatch in the browser process — a UAF read/vcall.
- reality — Speculative only. Post-patch the checked list turns any such case into a CHECK, denying an exploitation primitive.
Detection & hunting
For defenders and detection engineers:
- Browser-process crashes in RenderFrameMetadataProviderImpl dispatch — On unpatched builds, watch for crashes iterating observers_ with poisoned/dangling pointers; on patched builds these become CHECK failures with clear stacks.
- ObserverList checked-teardown CHECKs — Post-patch CHECK hits indicate a consumer with incorrect unregistration ordering — a real bug to file, not noise.
Audit directions
- Remaining Unchecked observer lists — Grep content/ and components/ for UncheckedAndDanglingUntriaged; each is an untriaged candidate for the same conversion, especially browser-process broadcasters.
- RenderFrameMetadataProvider consumers — Enumerate all AddObserver callers and confirm each RemoveObserver runs before the observer is destroyed; verify no observer is owned by an object destroyed during frame-submission callbacks.