ab488735a47d06ba5753be00ae0212f76e20a5ff  Data race in Range::visitNodesConcurrently during GC, leading to a use-after-free of RangeBoundaryPoint container nodes

The bug at a glance

This is a cross-thread data race on the boundary-point container nodes of live DOM Ranges: the JavaScriptCore GC thread reads m_start/m_end while the main thread mutates them from ordinary DOM APIs (setStart/setEnd/collapse/selectNodeContents/text mutations), racing on the Ref<Node> pointers and their refcounts. The consequence is a use-after-free of container Node objects in WebContent, reachable from unprivileged script on any page that holds a Range and provokes GC, which matches the assigned High / CVSS 8.1 given the powerful renderer-memory-corruption impact tempered by the timing-race attack complexity. This is the first of a related pair; commit 621e3bf30e is the follow-up that closes a residual race this patch leaves in setStart/setEnd.

DOM Ranges are visited concurrently by the garbage collector so it can mark the nodes they keep alive — and for years that visit read the range’s start and end boundary points straight off the object with zero synchronisation against the main thread that constantly rewrites them. A boundary point is a Ref<Node> plus offset; swapping one is a non-atomic pointer store paired with refcount churn, so the GC thread could observe a half-swapped container or race the refcount and mark (or fail to mark) the wrong node, ending in a use-after-free of the container Node. The fix is refreshingly blunt: add a per-Range Lock and hold it around every boundary-point mutation and around the GC visit, and protect each Range with a Ref while Document iterates its m_ranges. It is a textbook ‘we forgot the GC runs on another thread’ bug.

Root cause

Every live WebCore::Range registers with its Document and participates in garbage collection through Range::visitNodesConcurrently (renamed by this patch to Range::visitNodesInGCThread). That method runs on a JSC GC/marking thread and calls addWebCoreOpaqueRoot(visitor, m_start.container()) and addWebCoreOpaqueRoot(visitor, m_end.container()) to keep the range’s boundary container nodes alive across collection. Crucially it reads m_start and m_end — each a RangeBoundaryPoint wrapping a Ref<Node> — with no lock.

At the same time, the main thread freely mutates those same members through the normal live-Range API: Range::setStart, Range::setEnd, Range::collapse, Range::selectNodeContents, and the tree-mutation callbacks nodeChildrenChanged, nodeChildrenWillBeRemoved, nodeWillBeRemoved, textInserted, textRemoved, textNodesMerged and textNodeSplit. Each of these rewrites m_start and/or m_end — replacing the Ref<Node> container, which is a non-atomic pointer store plus a ref()/deref() pair on the old and new nodes.

The result is an unsynchronised read/write data race on the boundary-point container pointers and their reference counts. The GC thread can read a torn or stale container pointer, or interleave with the main thread’s deref() so that a container Node’s refcount is corrupted — dropping to zero and freeing a node that is still referenced, or failing to root a node the collector then reclaims. Either way the collector or a subsequent access touches a freed container Node: a use-after-free whose root cause is the missing memory model between the marking thread and DOM mutation.

This patch introduces mutable Lock m_boundaryPointLock as a member of Range (Range.h) and wraps every boundary-point mutation and the GC visit in Locker locker { m_boundaryPointLock }. Now the GC thread’s reads of m_start.container()/m_end.container() in visitNodesInGCThread are mutually exclusive with the main thread’s rewrites, so the collector always sees a consistent, fully-published pair of Ref<Node> containers with intact refcounts. Separately, Document::textInserted/textRemoved are changed to iterate for (Ref range : m_ranges) instead of range.get(), protecting each Range with a strong reference for the duration of the call so a Range cannot be destroyed out from under the iteration; correspondingly the NODELETE annotations are dropped from nodeChildrenChanged, textInserted and textRemoved in Range.h.

Key code

The GC-thread visit and a representative mutation now share m_boundaryPointLock (Range.cpp)

void Range::visitNodesInGCThread(JSC::AbstractSlotVisitor& visitor) const
{
    Locker locker { m_boundaryPointLock };
    addWebCoreOpaqueRoot(visitor, m_start.container());
    addWebCoreOpaqueRoot(visitor, m_end.container());
}

void Range::nodeWillBeRemoved(Node& node)
{
    ASSERT(&node.document() == m_ownerDocument.ptr());
    ASSERT(&node != m_ownerDocument.ptr());
    ASSERT(node.parentNode());

    Locker locker { m_boundaryPointLock };
    boundaryNodeWillBeRemoved(m_start, node);
    boundaryNodeWillBeRemoved(m_end, node);
    m_didChangeForHighlight = true;
}

Patch walkthrough

• Source/WebCore/dom/Range.h — Adds #include <wtf/Lock.h> and <wtf/Locker.h> and declares mutable Lock m_boundaryPointLock; alongside m_start and m_end. mutable is required because visitNodesInGCThread is const yet must take the lock. It also removes the NODELETE annotations from nodeChildrenChanged, textInserted and textRemoved, reflecting that these paths (now taking a lock and, in Document, iterating protected Refs) are no longer asserted delete-free.
• Source/WebCore/dom/Range.cpp — Wraps each boundary-point mutation in a Locker locker { m_boundaryPointLock } scope: setStart, setEnd, collapse, selectNodeContents, and the mutation callbacks nodeChildrenChanged, nodeChildrenWillBeRemoved, nodeWillBeRemoved, textInserted, textRemoved, textNodesMerged and textNodeSplit. It renames visitNodesConcurrently to visitNodesInGCThread and takes the same lock there before reading m_start.container()/m_end.container(). In setStart/setEnd the lock is taken twice in two separate scopes (once for the m_start/m_end.set(), once for the treeOrder re-order) — the seam that commit 621e3bf30e later closes.
• Source/WebCore/dom/Document.cpp — Changes Document::textInserted and Document::textRemoved to iterate for (Ref range : m_ranges) and call range->... instead of for (auto& range : m_ranges) range.get().... Taking a Ref per iteration keeps each Range alive across the callback, so a Range destroyed as a side effect of the text mutation cannot be used-after-free during the loop.

Background

RangeBoundaryPoint — A Range’s start/end position: a Ref<Node> container plus an offset (and a cached childBefore). Reassigning one is a non-atomic pointer store with ref()/deref() on the involved nodes, not a single atomic operation.

Concurrent GC marking in JavaScriptCore — JSC marks the heap on dedicated GC threads that run alongside the main thread. WebCore objects expose opaque-root visitors (here Range::visitNodesInGCThread) so the collector can keep DOM nodes reachable from live JS wrappers alive; these run off the main thread.

addWebCoreOpaqueRoot — Registers a WebCore object (here each boundary container Node) as a root with the slot visitor so it survives collection. If it reads a stale/freed pointer, the collector’s own bookkeeping touches freed memory.

WTF Lock / Locker — WebKit’s lightweight mutex and RAII scoped guard. Adding mutable Lock m_boundaryPointLock and taking a Locker in both the mutators and the const GC visit establishes the missing happens-before relationship between the two threads.

Vulnerability window

1. Range goes live — Script creates a Range (or a Selection-backed range) that attaches to its Document and becomes eligible for concurrent GC visiting.
2. GC visit begins — A JSC marking thread calls Range::visitNodesConcurrently and starts reading m_start.container()/m_end.container() without a lock.
3. Main thread mutates the boundary — Concurrently, script calls setStart/setEnd/collapse/selectNodeContents or triggers a DOM mutation callback, rewriting the Ref<Node> container and touching its refcount.
4. Race on the container Ref — The two threads interleave on the container pointer and refcount: the collector reads a torn pointer or races the deref(), corrupting the node’s reference count or rooting the wrong object.
5. Use-after-free — A container Node is freed while still referenced (or reclaimed by the collector), and a subsequent access from the collector or DOM dereferences freed memory — a UAF in WebContent.

Proof of concept

There is no deterministic trigger — the fix itself ships with ‘No new tests since there is no reliable way of testing this data race.’ The realistic approach, grounded in the patch, is to keep one live Range and hammer its boundary-point mutators (setStart/setEnd/selectNodeContents/collapse) while generating GC pressure so a JSC marking thread is concurrently inside visitNodesConcurrently reading m_start/m_end. A hit manifests as a WebContent ASan/TSan report on a Node refcount or container pointer, not as any script-visible result.

// Non-deterministic: the shipped patch notes there is no reliable way to test
// this data race. This stresses the racing window rather than guaranteeing a hit.
const r = document.createRange();
const a = document.body.appendChild(document.createElement('div'));
const b = document.body.appendChild(document.createElement('div'));

function churn() {
  for (let i = 0; i < 20000; i++) {
    // Rapidly rewrite m_start / m_end container Refs on the main thread...
    r.setStart(a, 0);
    r.setEnd(b, 0);
    r.selectNodeContents(i & 1 ? a : b);
    r.collapse(true);
    // ...while forcing allocation pressure so a concurrent GC marking thread
    // is likely to be visiting this Range's boundary points at the same time.
    if ((i & 0x3ff) === 0) { let junk = []; for (let j = 0; j < 5000; j++) junk.push({x:j}); }
  }
  requestAnimationFrame(churn);
}
churn();

Exploitation

1. Win the marking-thread race — The attacker must have the GC thread inside visitNodesInGCThread at the same instant the main thread rewrites a boundary container. This is a genuine data race, so it is probabilistic and sensitive to GC scheduling; TSan flags it readily but a wild free requires repeated attempts under allocation pressure.
2. Turn the race into a controlled free — Steering the refcount corruption toward freeing a specific attacker-useful container Node (rather than a random crash) is hard: the outcome depends on exact interleaving of ref()/deref() and the collector’s rooting, giving limited control over which node dies.
3. Reclaim and corrupt — Once a container Node is freed while still referenced, standard renderer UAF technique applies — reclaim the slot with an attacker-controlled object and abuse the dangling Node — but the non-deterministic trigger makes reliable weaponisation substantially harder than a deterministic single-threaded UAF.

Detection & hunting

For defenders and SOC / detection engineers:

• TSan data race on RangeBoundaryPoint / Node refcount — ThreadSanitizer reports where one stack is a JSC marking thread in Range::visitNodesConcurrently/visitNodesInGCThread and the other is a main-thread Range mutator touching m_start/m_end are the direct signature.
• GC-thread crash rooting a Range container — ASan use-after-free/heap-corruption crashes whose backtrace passes through addWebCoreOpaqueRoot from a Range visit during marking indicate a freed container node.
• Range-heavy pages under GC pressure — Fuzz with long-lived Ranges (or Selections) whose boundaries are mutated in tight loops while forcing collections; instrument for concurrent access to m_start/m_end.

Audit directions

• Other DOM objects visited on the GC thread — Audit every WebCore object exposing a concurrent GC visitor (opaque-root registration) for members mutated on the main thread without a lock — Selection, live NodeLists, highlights and similar boundary-holding types are prime candidates.
• Residual seams in this same patch — The setStart/setEnd here take the lock in two separate scopes with an unlocked treeOrder read of m_start/m_end between them; commit 621e3bf30e closes exactly this — verify no other mutator has the same split-lock pattern.
• Ref-vs-get iteration over range collections — Grep Document and friends for for (auto& x : m_ranges)/.get() iteration over collections whose elements can be destroyed by callbacks; each is a potential mid-iteration UAF like the one fixed in Document::textInserted/textRemoved.