Race condition in JSXPathResult::visitAdditionalChildren during GC

e8db86e

A confirmed concurrent GC UAF in XPath's node set handling. Before the fix, the GC thread iterated m_value.toNodeSet() directly while the main thread could trigger XPathNodeList::sort (e.g., via XPathNodeList::firstNode), reallocating the internal vector and invalidating the GC thread's iterator. The original FIXME comment literally read "This looks like it might race, but I'm not sure" — a known-unknown in shipping code.

Source/WebCore/xml/XPathResult.h

-    XPath::NodeSet m_nodeSet; // FIXME: why duplicate the node set stored in m_value?
+    Lock m_nodeSetLock;
+    XPath::NodeSet m_nodeSet WTF_GUARDED_BY_LOCK(m_nodeSetLock);

Source/WebCore/xml/XPathResult.cpp

+template<typename Visitor>
+void XPathResult::visitAdditionalChildrenInGCThread(Visitor& visitor)
+{
+    Locker locker { m_nodeSetLock };
+    for (auto& node : m_nodeSet)
+        addWebCoreOpaqueRoot(visitor, node.get());
+}

Significance

Concurrent GC UAFs are one of the highest-value vulnerability classes in browser engines — they require no timing-precise triggering from JS because the GC thread is always running.

Audit directions

snapshotItem() is visible in the diff and reads directly from m_value.toNodeSet(), not from the locked m_nodeSet — it therefore receives no protection from the new lock. If the main thread calls convertTo(ORDERED_NODE_SNAPSHOT_TYPE) concurrently with a GC-triggered sort, or if m_value's node set can be mutated while snapshotItem() is iterating, the same race class remains. The destructor asserts that m_nodeSet and m_value.toNodeSet() contain the same nodes — convertTo() handles multiple type transitions, and any path modifying m_value without updating m_nodeSet breaks the invariant. m_nodeSetLock is a plain (non-recursive) Lock; if any code path under the lock triggers a GC mark via a RefPtr destructor, the GC thread acquiring the same lock could deadlock. The FIXME predates this fix — search for other visitAdditionalChildrenInGCThread implementations across WebKit that iterate collections also mutated on the main thread.