a7c62a7cb94dda2dd38e75ec7c2d0c25a5d4a2de  Enforce EyeDropperChooserImpl liveness after eye_dropper_.reset()

The bug at a glance

Defense-in-depth hardening against a potential use-after-free, not a fix for a live bug: the commit message states EyeDropperChooserImpl remains alive under normal operation and adds a CHECK only in case destroying the UI widget ever triggers synchronous WebContents destruction. No current trigger is demonstrated, so impact today is a crash-on-detection (CHECK) rather than exploitable corruption.

A defender cares because it documents a latent UAF class: a DocumentService that continues using ’this’ after tearing down an owned UI widget, where widget destruction could re-enter and destroy the service’s owning context.

Root cause

ColorSelected()/ColorSelectionCanceled() call eye_dropper_.reset() (destroying the EyeDropper UI widget) and then continue to use the EyeDropperChooserImpl instance (ClearActiveEyeDropper(), run callback_). If widget destruction ever synchronously destroyed the WebContents/document, the EyeDropperChooserImpl (a DocumentService) could be freed, making the subsequent member accesses a use-after-free. The patch captures a WeakPtr before reset() and CHECK()s it afterward, converting any such reentrant self-destruction into a controlled crash instead of UAF.

Key code

WeakPtr liveness CHECK guards against reentrant self-destruction after widget reset

base::WeakPtr<EyeDropperChooserImpl> weak_this =
    weak_ptr_factory_.GetWeakPtr();
eye_dropper_.reset();
CHECK(weak_this);
ClearActiveEyeDropper();

Patch walkthrough

• content/browser/eye_dropper_chooser_impl.cc — In both result handlers, captures a WeakPtr to self before eye_dropper_.reset() and CHECKs it is still valid afterward, so that if destroying the UI widget ever re-enters and destroys the owning document/WebContents (freeing this DocumentService), the code fails fast instead of dereferencing freed memory in the following lines.
• content/browser/eye_dropper_chooser_impl.h — Adds base::WeakPtrFactory member (and weak_ptr.h include) to support the liveness check.

Background

EyeDropperChooserImpl — Browser-side DocumentService backing the EyeDropper API that owns an EyeDropper UI widget.

DocumentService lifetime — A DocumentService is tied to a RenderFrameHost/document and is destroyed when that document goes away.

Reentrant destruction — Tearing down UI can synchronously run code (e.g., focus/activation changes) that may destroy higher-level objects, a classic UAF source in browser code.

Vulnerability window

1. Reachability — A page opens the EyeDropper and the user selects or cancels a color, invoking ColorSelected/ColorSelectionCanceled.
2. Trigger — eye_dropper_.reset() destroys the UI widget; in a hypothetical reentrant case widget teardown destroys the WebContents (inferred, not shown to occur today).
3. Primitive — Subsequent member access on the freed EyeDropperChooserImpl would be a UAF (inferred, guarded against rather than demonstrated).
4. Escalation — Not established; the CHECK turns any such path into a safe crash (inferred).

Proof of concept

No concrete trigger exists in the diff; it is a speculative-hardening CHECK. Conceptually one would need a scenario where EyeDropper widget destruction synchronously tears down the owning WebContents during color selection, which the authors state does not happen in normal operation.

Exploitation

1. Hypothetical reach — Find an environment where widget teardown re-enters and destroys the document during color selection (inferred, not known to exist).
2. Use-after-free — Access freed EyeDropperChooserImpl members after reset (inferred); now prevented by the CHECK.

Detection & hunting

For defenders and detection engineers:

• weggli / grep for ‘reset()’ or owned-object destruction followed by use of ’this’ members without a WeakPtr guard — In content/browser DocumentService/mojo impls, flag handlers that destroy an owned UI object then keep touching instance state.
• CHECK(weak_this) pattern — Treat newly added post-reset liveness CHECKs as markers of latent reentrant-destruction UAF classes worth auditing nearby.

Audit directions

• content/browser DocumentService / mojo receiver impls that own UI widgets — Look for destroy-then-use sequences (reset/close of owned UI followed by callback execution) lacking WeakPtr liveness checks.
• Other choosers/pickers (color, file, date) in content/browser — Audit for the same reset-UI-then-use-self pattern.