[5] WebCore Range concurrent-GC UAF on lock-free treeOrder

Severity: High | Component: WebCore DOM Range | 621e3bf

Rated High because the diff confirms that Range::setStart/setEnd released m_boundaryPointLock between writing one boundary and reading both for treeOrder, exposing the concurrent GC marker to a torn RangeBoundaryPoint; the commit title attributes this directly to a UAF of container Nodes under Range::visitNodesConcurrently, which is a classic DOM-node UAF primitive in WebContent.

In Range::setStart and Range::setEnd, the patch collapses two separate m_boundaryPointLock critical sections into one. The pre-fix code locked once to write m_start (or m_end), released the lock, re-read both boundary points without the lock to compute treeOrder(...), then re-acquired the lock to fix the ordering by copying one boundary into the other. The new code computes the ordering question first using the parameters and a snapshot of the other boundary, then takes the lock once, writes the updated boundary point, and conditionally writes the second boundary point — all inside the same Locker { m_boundaryPointLock } scope. The seven boundary* static helpers are also rewritten to take a leading Locker<Lock>& parameter, documenting at the type level that the caller must already hold m_boundaryPointLock.

Source/WebCore/dom/Range.cpp

Patch Details

In Range::setStart and Range::setEnd, the pre-fix shape { lock; write_one; } read_unlocked treeOrder; { lock; maybe_write_other; } is replaced with compute_decision_from_args_and_snapshot; { lock; write_one; maybe_write_other; }. The conditional second boundary write now lives inside the same critical section as the first boundary write, so the marker cannot observe a partial update. Seven boundary* static helpers (boundaryNodeChildrenChanged, boundaryNodeChildrenWillBeRemoved, boundaryNodeWillBeRemoved, boundaryTextInserted, boundaryTextRemoved, boundaryTextNodesMerged, boundaryTextNodesSplit) take a leading Locker<Lock>& parameter; their call sites in Range::nodeChildrenChanged, Range::nodeChildrenWillBeRemoved, Range::nodeWillBeRemoved, Range::textInserted, Range::textRemoved, Range::textNodesMerged, and Range::textNodeSplit are updated to forward the existing locker. Only Range.cpp is touched.

Lock-free read of shared multi-word DOM boundary state races a concurrent GC visitor, allowing the marker to retain a stale container Node* past its lifetime.

Background

Range represents a live DOM range — a [start, end] interval over the DOM tree — used by Selection, find-in-page, Highlight, and similar features. Both endpoints are RangeBoundaryPoint values holding a RefPtr<Node> container, an unsigned offset, and a childBefore pointer used to keep the offset coherent across child mutations. The boundary points are multi-word state that cannot be read coherently without synchronization.

JSC's garbage collector runs a concurrent marking phase on a separate thread while JavaScript continues to execute on the main thread. Concurrent visitors call visit*Concurrently overrides on objects that hold native references to GC-managed roots, and these visitors must synchronize with mutator writes to any field they read. Range::visitNodesConcurrently is the per-Range concurrent visitor that traces m_start and m_end's container Nodes. m_boundaryPointLock is the per-Range lock that serializes mutator writes to m_start/m_end against this concurrent visitor.

treeOrder is a comparator that walks the DOM tree to order two boundary points; reading its inputs requires that those inputs be stable for the duration of the call. Locker<Lock>& as a parameter is a WebKit idiom that documents at the function signature that the caller must already hold the named lock — it does not enforce identity, but it forces every call site to materialize a locker, which is mechanically auditable at compile time.

Analysis

The bug is a data race producing a concurrent use-after-free during GC marking. Pre-fix Range::setStart/setEnd exposed two windows in which the GC's concurrent visitor could observe m_start/m_end in an inconsistent or torn state. After updating one boundary inside a lock and releasing it, the code re-read both m_start and m_end without the lock to compute treeOrder(makeBoundaryPoint(m_start), makeBoundaryPoint(m_end)). Because RangeBoundaryPoint is multi-word, that lock-free read races every concurrent writer and the GC marker. Between the first lock release and the conditional second lock re-acquisition, the marker could observe a state where m_start was already advanced past m_end, then the writer would overwrite m_end in a separate critical section.

Audit directions