c782b7694c1850a9595c5ce0c2397162b7eb85dc  [M145][trace] Hold PaintTimingVisualizer as unique_ptr to fix use-after-poison

The bug at a glance

This is a genuine memory-safety defect: a raw pointer to a PaintTimingVisualizer that lives inside Oilpan-managed (garbage-collected) memory is retained by a non-GC observer list and is dereferenced after Oilpan has poisoned that memory during collection. In principle a stale reference into freed/poisoned heap is the classic ingredient of a use-after-free and can, under the right allocator state, become a controllable corruption primitive. Two facts cap severity at medium rather than high. First, the object only exists while tracing is active: PaintTimingVisualizer is constructed only when PaintTimingVisualizer::IsTracingEnabled() returns true, i.e. when the disabled-by-default ’loading’ trace category (used by paint-timing debugging/visualization tooling) is enabled. That is not reachable by plain web content; it requires a local tracing session or a devtools/perfetto capture. Second, in Chrome’s Oilpan build ‘poisoning’ is primarily an ASan/heap-verification mechanism, so on production builds the immediate consequence is more often a controlled crash than a silently corrupted, attacker-shaped object. The assigned 6.5 reflects a real UAF-class flaw with a meaningfully constrained trigger surface.

The bug is only live when a paint-timing trace category is enabled, so it is not a drive-by web primitive. The realistic angle is (a) users/testers running with tracing on, or (b) any downstream code path that flips tracing on programmatically, where navigating or tearing down a frame while the trace observer list still references the poisoned visualizer produces a dangling access. The blamed CL (crrev 7234082) did not introduce the lifetime error; it merely increased how many notifications flow to trace observers, which made the pre-existing dangling reference actually get dereferenced.

Root cause

PaintTimingDetector is a garbage-collected Blink object (MakeGarbageCollected). It embedded its PaintTimingVisualizer by value inside a std::optional member, and PaintTimingVisualizer was marked DISALLOW_NEW (meaning it must be embedded in another object rather than heap-allocated on its own). Because the optional lived inside a GCed object, the visualizer’s storage was itself GCed memory. PaintTimingVisualizer also derives from trace_event::TraceSessionObserver and, at construction, registers itself into a process/thread-global trace observer list. That observer list is an ordinary (non-Oilpan) container and therefore holds a raw back-pointer into GCed memory. Oilpan explicitly forbids non-GC code from holding references into GCed objects and, before running destructors during a GC sweep, it poisons the object’s memory. The observer registration outlives the safe window: when notifications are dispatched (RecordMainFrameViewport / the various DumpXxxDebuggingRect callbacks routed through Visualizer()), or when the observer is finally unregistered in the destructor, the code touches memory that Oilpan has already poisoned -> use-after-poison. The fix moves the visualizer out of GCed storage by holding it via std::unique_ptr (a normal heap allocation not owned by Oilpan) and drops DISALLOW_NEW, so the observer list now points at plain heap memory whose lifetime is deterministically controlled by the unique_ptr rather than by GC sweeps.

Key code

paint_timing_detector.h: the visualizer is moved out of GCed storage into a unique_ptr so the trace observer list no longer references Oilpan memory.

// Because PaintTimingVisualizer is a TraceSessionObserver, unique_ptr is
// needed to avoid having a reference back into GCed memory, which is
// forbidden by oilpan.
std::unique_ptr<PaintTimingVisualizer> visualizer_;

// accessor:
PaintTimingVisualizer* Visualizer() { return visualizer_.get(); }

Patch walkthrough

• third_party/blink/renderer/core/paint/timing/paint_timing_detector.h — Core of the fix. The member changes from std::optional visualizer_ (value embedded in the GCed PaintTimingDetector) to std::unique_ptr visualizer_, with an explanatory comment that a TraceSessionObserver must not live in GCed memory. Visualizer() changes return type from std::optional&& to a raw PaintTimingVisualizer* (nullptr when absent), which is why every caller had to be updated.
• third_party/blink/renderer/core/paint/timing/paint_timing_visualizer.h — Removes the DISALLOW_NEW() macro from PaintTimingVisualizer. That macro previously constrained the class to be embedded inside another object; since it is now separately heap-allocated via unique_ptr, the constraint is both unnecessary and wrong.
• third_party/blink/renderer/core/paint/timing/paint_timing_detector.cc — Construction sites switch from visualizer_.emplace() to visualizer_ = std::make_unique() in both the constructor (when tracing is already on) and NotifyPaintFinished() (lazy creation when tracing turns on later). Behaviorally equivalent creation, but now allocating off the Oilpan heap.
• third_party/blink/renderer/core/paint/timing/image_paint_timing_detector.cc — Two call sites updated from if (std::optional<PaintTimingVisualizer>& visualizer = ...Visualizer()) to if (PaintTimingVisualizer* visualizer = ...Visualizer()) for the image debugging-rect dumps. Pointer-null semantics replace optional-truthiness; the guarded body is unchanged.
• third_party/blink/renderer/core/paint/timing/text_paint_timing_detector.cc — Same mechanical caller update for the text debugging-rect dump path (DumpTextDebuggingRect).

Background

Oilpan — Blink’s tracing garbage collector. Objects created via MakeGarbageCollected are managed by Oilpan; references between GCed objects use Member<>/Persistent<>. Non-GC code is not permitted to hold raw pointers into GCed heap because the collector may move/reclaim/poison that memory.

Poisoning / use-after-poison — Before running destructors during a sweep, Oilpan (with heap verification / ASan support) marks an object’s memory as poisoned so that any subsequent access is flagged. A pointer used after that point is a use-after-poison, the Oilpan analogue of a use-after-free.

TraceSessionObserver — A base class whose instances self-register into a global observer list to receive trace-session start/stop notifications. The list is plain heap storage, so any observer that lives inside GCed memory creates an illegal GC->non-GC back-reference.

DISALLOW_NEW() — A Blink macro asserting a class must be embedded within another allocation rather than heap-allocated on its own. It was appropriate when the visualizer was an inline std::optional member but blocks the unique_ptr fix, so it is removed.

PaintTimingVisualizer / IsTracingEnabled() — A debugging aid that emits paint-timing rectangles into trace events for third-party visualization tools. It is only instantiated when the relevant trace category is enabled, which gates the entire bug behind an active tracing session.

Vulnerability window

1. Latent — The visualizer was embedded as an optional inside the GCed PaintTimingDetector while also being a self-registering TraceSessionObserver, creating an illegal GC->non-GC back-reference that was dormant while few notifications were dispatched.
2. Trigger amplification — CL 7234082 increased the number of notifications sent to trace observers, so the dangling reference began to actually be dereferenced during real sessions, surfacing the crash.
3. Discovery — Local testing under Oilpan heap verification showed the PaintTimingVisualizer pointer poisoned before its destructor ran (crbug.com/475613896).
4. Fix — Landed on main as 209f799f3e (#1569820), then cherry-picked to the 7632 branch as c782b7694c for the M145 stable/beta line.

Proof of concept

No proof-of-concept or regression test ships with this diff. Reproduction is not driven by page content alone: an Oilpan build with heap verification/ASan and the paint-timing trace category enabled is required, after which navigating or destroying a frame view while the trace observer list still references the visualizer surfaces the poisoned access. I will not fabricate exploit code; the honest characterization is a tracing-gated dangling reference that manifests as a use-after-poison crash under verification builds.

Exploitation

1. Precondition — A paint-timing trace category must be enabled so a PaintTimingVisualizer is instantiated and registered as a TraceSessionObserver. Without an active tracing session the vulnerable object never exists.
2. Dangling window — During GC of the owning PaintTimingDetector, Oilpan poisons the visualizer’s storage before its destructor unregisters the observer. The non-GC observer list still holds the raw pointer during this window.
3. Dereference — A trace notification dispatch or the observer’s own teardown touches the poisoned memory. On production builds this is typically an unstable read; a practical corruption primitive would require defeating Oilpan reclamation timing, making weaponization impractical from the web.

Detection & hunting

For defenders and detection engineers:

• Oilpan use-after-poison / ASan reports —
• Correlation with tracing sessions —

Audit directions

• Other GC-embedded observers —
• DISALLOW_NEW + self-registering types —
• std::optional members inside GCed classes —