209f799f3e3e5f32cb6d4756481c9eaaaffd7478  [trace] Hold PaintTimingVisualizer as unique_ptr to fix use-after-poison

The bug at a glance

A genuine memory-safety defect: an off-heap observer list (the trace-session observer registry) held a raw pointer to a PaintTimingVisualizer that was DISALLOW_NEW-embedded inside a garbage-collected PaintTimingDetector. Oilpan poisons GCed memory before running destructors, so the observer registry could dereference the visualizer after its backing store was poisoned — a use-after-poison, the sanitizer-detected precursor to a real use-after-free. That is why it is not low. It is not high because reachability is gated behind tracing being enabled (PaintTimingVisualizer::IsTracingEnabled()): the visualizer is only constructed when a trace session for the relevant category is running, so an attacker cannot reach the poisoned deref from ordinary web content without a tracing session active on the victim’s machine. The author marks exploitable=false and notes the blamed CL merely increased observer notification frequency, exposing a latent lifetime bug rather than creating an attacker primitive. Poisoned-memory access in the GC heap with attacker-influenceable timing, but behind a non-default tracing gate, is an honest medium / 6.5 — and squarely watch-tier: gated, no demonstrated exploit, but a real Oilpan ownership violation.

The core lesson is an Oilpan ownership rule: non-GC code may not hold a bare reference into GCed memory. PaintTimingVisualizer is a trace_event::TraceSessionObserver; when constructed it registers itself with the (non-GC, process-global) trace-session observer list, which stores a raw ’this’. But the visualizer lived as a std::optional member of PaintTimingDetector, a GCed object — so ’this’ pointed into GC-managed memory. Oilpan poisons a GCed object’s storage before its destructor runs; because the visualizer was embedded (DISALLOW_NEW) inside that storage, the observer list’s raw pointer referenced poisoned bytes, and any late trace notification dereferenced them. The fix moves the visualizer out of the GC heap into a std::unique_ptr member, so the address the observer list holds is ordinary heap memory with a deterministic C++ destructor, never poisoned by Oilpan. The angle for auditors: every TraceSessionObserver / base observer that is embedded in or owned by a GCed object.

Root cause

PaintTimingDetector (GCed, MakeGarbageCollected) owned its visualizer as std::optional visualizer_ and constructed it via visualizer_.emplace() when PaintTimingVisualizer::IsTracingEnabled(). PaintTimingVisualizer derives from trace_event::TraceSessionObserver and, on construction, adds itself (raw this) to the process-wide trace-session observer list; that list is not on the Oilpan heap. Because the optional stores the visualizer inline inside the GCed PaintTimingDetector allocation (and the class was tagged DISALLOW_NEW), the raw this recorded by the observer list points into GC memory. When the PaintTimingDetector is collected, Oilpan poisons the object’s memory region before invoking destructors (a sanitizer trap to catch dangling references into GC memory). If a trace notification is delivered to observers in that window — or, per the commit, simply because a blamed CL (crrev 7234082) increased how often observers are notified — the observer machinery dereferences the poisoned visualizer pointer, producing an ASAN use-after-poison report. The fix: (1) change the member to std::unique_ptr visualizer_, constructed with std::make_unique; the visualizer now lives in a normal heap allocation whose address is stable and whose destructor runs deterministically (unregistering the observer) without Oilpan poisoning. (2) Remove DISALLOW_NEW() from PaintTimingVisualizer, since it is no longer embedded in a GCed object. (3) Change the accessor Visualizer() from returning std::optional& to PaintTimingVisualizer* (visualizer_.get()), and update all four call sites (image/text detectors) from ‘std::optional& visualizer = …’ to ‘PaintTimingVisualizer* visualizer = …’, with the null-checked pointer semantics unchanged. The header comment records the rationale: a TraceSessionObserver must not be referenced back into GCed memory.

Key code

paint_timing_detector.h — move the observer off the GC heap into a unique_ptr

-  std::optional<PaintTimingVisualizer>& Visualizer() { return visualizer_; }
+  PaintTimingVisualizer* Visualizer() { return visualizer_.get(); }
 ...
-  std::optional<PaintTimingVisualizer> visualizer_;
+  // 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_;

// paint_timing_detector.cc — construct off-heap
-    visualizer_.emplace();
+    visualizer_ = std::make_unique<PaintTimingVisualizer>();

// paint_timing_visualizer.h — no longer embedded in a GCed object
 class CORE_EXPORT PaintTimingVisualizer
     : public trace_event::TraceSessionObserver {
-  DISALLOW_NEW();

Patch walkthrough

• third_party/blink/renderer/core/paint/timing/paint_timing_detector.h — The heart of the fix: visualizer_ becomes std::unique_ptr instead of std::optional embedded inline, and Visualizer() returns a raw pointer. Comment documents that a TraceSessionObserver must not live in GCed memory. This relocates the address held by the trace-session observer list off the Oilpan heap.
• third_party/blink/renderer/core/paint/timing/paint_timing_detector.cc — Both construction sites (constructor and NotifyPaintFinished lazy-init) switch from visualizer_.emplace() to std::make_unique(). The tracing-enabled gate (IsTracingEnabled) is unchanged, confirming the code only ever runs with an active trace session.
• third_party/blink/renderer/core/paint/timing/paint_timing_visualizer.h — Removes DISALLOW_NEW() from PaintTimingVisualizer. DISALLOW_NEW marked it as intended to be embedded within another object (here the GCed detector); that embedding was exactly the bug, so it is dropped now that the object is heap-allocated via unique_ptr.
• third_party/blink/renderer/core/paint/timing/image_paint_timing_detector.cc / text_paint_timing_detector.cc — Three call sites updated from ‘if (std::optional& visualizer = …Visualizer())’ to ‘if (PaintTimingVisualizer* visualizer = …Visualizer())’. Purely the accessor-type change following the member-type change; the null-check and DumpImageDebuggingRect/DumpTextDebuggingRect usage are identical. No behavioral change beyond the ownership fix.

Background

Oilpan poisoning — Blink’s garbage collector poisons a GCed object’s memory before running its destructor and reclaiming it, so any dangling reference into GC memory is caught by ASAN as a use-after-poison. It is a deliberate safety trap enforcing that non-GC code must not hold raw pointers into the GC heap.

DISALLOW_NEW — A Blink annotation meaning an object is meant to be embedded inline within another (often GCed) object rather than heap-allocated on its own. Here it caused PaintTimingVisualizer to live inside the GCed PaintTimingDetector allocation, which is what put the observer’s this-pointer in GC memory.

TraceSessionObserver — An observer interface for trace-session start/stop; instances register themselves in a process-global (non-GC) observer list. Holding a GC-memory address in that list is the forbidden cross-heap reference this CL removes.

PaintTimingVisualizer / IsTracingEnabled gate — A debugging/visualization helper that dumps paint-timing rects into traces for third-party tooling. It is only instantiated when tracing for the category is enabled, which gates reachability of the bug to sessions with tracing active.

Blamed CL 7234082 — The commit message states this CL didn’t introduce the bug but exposed it by sending more notifications to observers, increasing the chance a notification lands during the post-poison, pre-destruct window.

Vulnerability window

1. latent — PaintTimingVisualizer was embedded (std::optional, DISALLOW_NEW) inside the GCed PaintTimingDetector and registered as a non-GC TraceSessionObserver — a latent cross-heap reference from the start.
2. exposure — crrev.com/7234082 increased observer notification frequency, making the post-poison dereference observable in local testing / sanitizer runs.
3. report — crbug.com/475613896: pointer to PaintTimingVisualizer poisoned before its destructor runs; observer list dereferences poisoned GC memory.
4. fix — 2026-01-15, Etienne Pierre-Doray lands 209f799f3e (m@{#1569820}): move the visualizer to a unique_ptr off the GC heap, drop DISALLOW_NEW, adjust accessor and call sites.

Proof of concept

No test or exploit ships with this commit; it is an ownership/lifetime correction found via ASAN in local testing, per the commit message. No exploit is demonstrated and none is invented. Honest reproduction recipe: build Chromium with ASAN, start a tracing session on the paint-timing/loading category so PaintTimingVisualizer::IsTracingEnabled() is true and a visualizer is constructed for a frame’s PaintTimingDetector, then drive navigation/frame teardown so the GCed PaintTimingDetector is collected while trace notifications are still being delivered to the observer list. Pre-fix, ASAN should flag a use-after-poison read when the observer machinery dereferences the visualizer’s this after Oilpan poisons the detector’s storage. Reachability requires an active trace session, so this is not reachable from unprivileged web content alone; that gating is why it is honestly watch-tier rather than a shippable web exploit. Because the primitive demonstrated is a poisoned-memory read with no shown attacker control over contents, no PoC exploit code is warranted.

Exploitation

1. precondition — A trace session covering the paint-timing category must be active so a PaintTimingVisualizer exists (IsTracingEnabled gate). This is not the default state for ordinary browsing, sharply limiting attacker reach.
2. trigger — Cause the GCed PaintTimingDetector to be collected (frame/document teardown) while the trace-session observer list still holds and notifies the embedded visualizer’s pointer — a lifetime race widened by the increased notification frequency from the blamed CL.
3. reached primitive — Dereference of Oilpan-poisoned memory (use-after-poison), the sanitizer-caught form of a UAF read. No write primitive or attacker-controlled contents are demonstrated in the diff; realistic impact is a crash/UAF read, not shown RCE.

Detection & hunting

For defenders and detection engineers:

• ASAN use-after-poison in trace-session observer dispatch —
• Renderer crashes correlated with active tracing —

Audit directions

• GCed objects owning TraceSessionObservers / base observers —
• Other std::optional members inside GCed classes where T registers ’this’ externally —
• Notification-frequency regressions surfacing latent lifetime bugs —