a926a679cf Crash in -[WebScrollbarPartAnimationMac setCurrentProgress:]
Triage note: Converts raw CheckedPtr to ThreadSafeWeakPtr and null-checks before deref in an async animation callback, fixing a use-after-free.
Contents
The bug at a glance
High. A ScrollerMac object owned by ScrollerPairMac is destroyed on the main thread, while an in-flight NSAnimation display-link callback on the ‘WebCore: Scrolling’ thread still holds a CheckedPtr to it and calls updateProgress in -[WebScrollbarPartAnimationMac setCurrentProgress:]. This is a cross-thread use-after-free of a heap object. Reachability is via ordinary scrollbar animation, so it is broadly triggerable; confidence 0.8 as it is a race and non-web-content thread interaction.
The animation objects (WebScrollbarPartAnimationMac / WebScrollerImpDelegateMac) held ScrollerMac by CheckedPtr, which enforces no lifetime; it only asserts. When ~ScrollerPairMac freed its scrollers on the main thread, an already-scheduled animation callback firing on the scrolling thread would deref the freed ScrollerMac. CheckedPtr gives detection in debug but no protection in release.
Root cause
ScrollerPairMac owns two ScrollerMac instances (vertical and horizontal) and drives NSAnimation-based scrollbar fade/expansion animations through Objective-C helper objects: WebScrollbarPartAnimationMac (a subclass of NSAnimation) and WebScrollerImpDelegateMac. These helpers stored a CheckedPtr<WebCore::ScrollerMac> _scroller. CheckedPtr provides use-after-free assertions in debug builds but does not keep the pointee alive.
NSAnimation progress callbacks are delivered via a display link on the ‘WebCore: Scrolling’ thread. -[WebScrollbarPartAnimationMac setCurrentProgress:] calls CheckedPtr { _scroller }->updateProgress(…). Meanwhile, the owning ScrollerMac (and its ScrollerPairMac) is destroyed on the main thread by ~ScrollerPairMac. Because the scrolling-thread callback can already be in flight when the main-thread destructor runs, setCurrentProgress: can dereference a ScrollerMac that has just been freed, a cross-thread use-after-free crashing in updateProgress.
The fix converts ScrollerMac from a plain fast-allocated CanMakeThreadSafeCheckedPtr type into a ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<ScrollerMac, WTF::DestructionThread::Main>, TZone-allocated, with a private constructor and a static create() returning Ref<ScrollerMac>. The animation helpers now hold ThreadSafeWeakPtr<WebCore::ScrollerMac> _scroller instead of CheckedPtr. In setCurrentProgress: the code first does ‘RefPtr scroller = _scroller.get();’ and returns early if null, only dereferencing through the RefPtr, so the callback either gets a strong reference that keeps ScrollerMac alive for the duration of the call, or observes that it is gone and safely no-ops. The same get()+null-check-then-RefPtr pattern is applied across every delegate method (mouseLocationInScrollerForScrollerImp, effectiveAppearance, animateKnobAlphaTo/animateTrackAlphaTo, animateUIStateTransition, animateExpansionTransition, setUpAlphaAnimation, invalidate sets _scroller = nullptr).
DestructionThread::Main guarantees ScrollerMac is always torn down on the main thread even if the last Ref is dropped on the scrolling thread, and ~ScrollerMac now ASSERTs isMainThread(). ScrollerPairMac holds the scrollers as const Ref<ScrollerMac> (was UniqueRef), and its destructor’s ensureOnMainThread block no longer needs to capture the scrollerImps via takeScrollerImp() because ScrollerMac lifetime is now managed by refcount with main-thread destruction. Additional null guards for m_pair (a ThreadSafeWeakPtr back to the pair) are added in lastKnownMousePositionInScrollbar, visibilityChanged and updateMinimumKnobLength, since a RefPtr from an in-flight callback can briefly outlive the pair.
Key code
setCurrentProgress: takes a strong RefPtr and null-checks before deref (ScrollerMac.mm)
[super setCurrentProgress:progress];
+ RefPtr scroller = _scroller.get();
+ if (!scroller)
+ return;
+
CGFloat currentValue;
if (_startValue > _endValue)
currentValue = 1 - progress;
else
currentValue = progress;
- CheckedPtr { _scroller }->updateProgress(_featureToAnimate, currentValue);
+ scroller->updateProgress(_featureToAnimate, currentValue);
Patch walkthrough
Source/WebCore/page/scrolling/mac/ScrollerMac.h— ScrollerMac’s base changes from CanMakeThreadSafeCheckedPtr<ScrollerMac> (with WTF_OVERRIDE_DELETE_FOR_CHECKED_PTR and fast-allocation) to ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<ScrollerMac, WTF::DestructionThread::Main>, TZone-allocated. The public constructor is replaced by a static create() factory returning Ref<ScrollerMac>, with the constructor made private.Source/WebCore/page/scrolling/mac/ScrollerMac.mm— The Obj-C helpers’ _scroller ivars change from CheckedPtr to ThreadSafeWeakPtr. Every access becomes ‘RefPtr scroller = _scroller.get();’ with a null guard before dereference; setCurrentProgress: returns early if the scroller is gone instead of dereferencing a raw CheckedPtr. create() and WTF_MAKE_TZONE_ALLOCATED_IMPL are added, ~ScrollerMac gains ASSERT(isMainThread()), and lastKnownMousePositionInScrollbar/visibilityChanged/updateMinimumKnobLength gain null checks on RefPtr pair = m_pair.get().Source/WebCore/page/scrolling/mac/ScrollerPairMac.h/.mm— m_verticalScroller/m_horizontalScroller change from const UniqueRef<ScrollerMac> to const Ref<ScrollerMac>, constructed via ScrollerMac::create(*this, …). The delegate now holds scrollers as Ref<ScrollerMac>. ~ScrollerPairMac’s ensureOnMainThread block is simplified to only capture m_scrollerImpPair, dropping the takeScrollerImp() captures now that refcounting handles scroller teardown.Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm— updateFromStateNode changes CheckedRef horizontalScroller/verticalScroller to Ref, matching the new ref-counted ScrollerMac ownership model.
Background
CheckedPtr vs ThreadSafeWeakPtr — CheckedPtr asserts (debug only) that a pointee is still alive but does not manage lifetime; ThreadSafeWeakPtr can be upgraded to a RefPtr atomically, keeping the object alive for the duration of use or safely returning null.
WTF::DestructionThread::Main — A ThreadSafeRefCounted policy ensuring the object’s destructor runs on the main thread even when the final deref happens on another thread, avoiding main-thread-only teardown races.
WebCore: Scrolling thread — A dedicated thread where scrolling-tree/NSAnimation display-link callbacks execute, distinct from the main thread that owns ScrollerPairMac.
NSAnimation setCurrentProgress: — Called repeatedly by the animation runloop/display link to advance scrollbar fade/expansion; here it forwarded to ScrollerMac::updateProgress on the scrolling thread.
Vulnerability window
- Animation scheduled — A scrollbar fade/expansion animation is running, its WebScrollbarPartAnimationMac holding a CheckedPtr to ScrollerMac.
- Teardown — On the main thread ~ScrollerPairMac destroys the ScrollerMac instances (UniqueRef).
- Callback in flight — A display-link setCurrentProgress: callback is already executing on the scrolling thread.
- UAF — setCurrentProgress: dereferences the freed ScrollerMac via CheckedPtr, crashing in updateProgress.
- Fix — ScrollerMac becomes ThreadSafeRefCounted; the callback upgrades a ThreadSafeWeakPtr to RefPtr, keeping it alive or no-oping if gone.
- Teardown discipline — DestructionThread::Main + ASSERT(isMainThread()) guarantee correct-thread destruction.
Triggering
No test added; the commit says ‘Covered by existing scrollbar tests.’ Trigger: on macOS, run a scrollbar fade/expansion animation (e.g. an overlay scrollbar mid fade) and destroy the associated ScrollerPairMac (navigate away / destroy the scrolling node) so ~ScrollerPairMac frees the ScrollerMac on the main thread while a WebScrollbarPartAnimationMac setCurrentProgress: display-link callback is still in flight on the WebCore: Scrolling thread; the callback then dereferences the freed ScrollerMac. Requires winning the cross-thread race between animation callback and destruction.
Exploitation
- Race setup — Induce continuous scrollbar animations (overlay scrollbar fades) then rapidly tear down the scroller (remove the scrollable element / navigate) to overlap destruction with an in-flight callback.
- Reclaim — After ScrollerMac is freed, allocate same-size objects on the scrolling/main thread to control the freed slot before setCurrentProgress: derefs it.
- Control flow — updateProgress reads members and may call through pointers on the reclaimed ScrollerMac; a controlled reallocation can yield a vtable/function-pointer hijack or type confusion.
- Constraint — This is a timing-sensitive cross-thread race not driven purely by web content timing, so reliability requires careful animation/teardown orchestration.
Detection & hunting
For defenders and SOC / detection engineers:
- Crash in -[WebScrollbarPartAnimationMac setCurrentProgress:] / updateProgress —
- CheckedPtr across threads —
Audit directions
- Async callbacks holding CheckedPtr —
- Destruction-thread correctness —
- ScrollerPairMac lifetime —