a814080321  Potential use after free of m_responseDocument in XMLHttpRequest::visitAdditionalChildren()

The bug at a glance

An XMLHttpRequest is script-reachable on every web page and its wrapper is visited concurrently by the JSC GC (marking) thread, so the race is trivially reachable by untrusted content. A RefPtr<Document> that the GC thread reads while the main thread nulls it out is a classic cross-thread use-after-free of a large, exploitable DOM object graph, warranting the high rating even though the window is narrow.

visitAdditionalChildrenInGCThread runs on the GC thread and dereferenced m_responseDocument / m_upload without any synchronization while the main thread could reassign or null those members. The fix introduces a single m_gcLock guarding both members and funnels every producer and every consumer (including the GC visitor) through it.

Root cause

WebKit’s concurrent GC visits opaque roots for a wrapper on a dedicated marking thread while the main thread keeps executing JavaScript. JSXMLHttpRequest::visitAdditionalChildrenInGCThread previously called wrapped().optionalUpload() and wrapped().optionalResponseXML(), each of which is a bare getter returning m_upload.get() / m_responseDocument.get(), and then passed the raw pointer to addWebCoreOpaqueRoot. Nothing prevented the main thread from concurrently mutating those RefPtrs.

The m_responseDocument RefPtr is written on the main thread from several paths: responseXML() lazily builds or discards the document (m_responseDocument = nullptr on the not-well-formed / text-html branches, or assigns the freshly parsed Document), and clearResponseBuffers() resets it to nullptr when a new request begins. If the main thread runs the m_responseDocument = nullptr assignment while the GC thread has just loaded the old pointer, the RefPtr’s destructor can drop the last reference and free the Document, after which the GC thread calls addWebCoreOpaqueRoot on freed memory - a read (and marking write into the freed cell’s header) of a dangling Document.

The same hazard exists for the m_upload unique_ptr, read by the GC visitor and by updateHasRelevantEventListener/didSendData/dispatchErrorEvents, while upload() lazily initializes it.

The patch adds a WTF Lock m_gcLock and annotates both m_upload and m_responseDocument with WTF_GUARDED_BY_LOCK(m_gcLock). Every site that reads or writes these members now takes Locker locker { m_gcLock }, and the GC-thread visitor logic is moved out of the binding into XMLHttpRequest::visitAdditionalChildrenInGCThread which acquires the same lock before touching either member. Main-thread consumers that need to call out (e.g. dispatchProgressEvent) snapshot m_upload into a RefPtr under the lock and then release it before the call, so the object is kept alive across the unlocked call while the lock is not held during reentrant script.

Key code

New lock-guarded GC visitor in XMLHttpRequest.cpp

template<typename Visitor>
void XMLHttpRequest::visitAdditionalChildrenInGCThread(Visitor& visitor)
{
    Locker locker { m_gcLock };
    if (m_upload)
        addWebCoreOpaqueRoot(visitor, *m_upload);

    SUPPRESS_UNCHECKED_LOCAL if (auto* document = m_responseDocument.get())
        addWebCoreOpaqueRoot(visitor, *document);
}

DEFINE_VISIT_ADDITIONAL_CHILDREN_IN_GC_THREAD(XMLHttpRequest);

Patch walkthrough

• Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp — JSXMLHttpRequest::visitAdditionalChildrenInGCThread no longer reaches into optionalUpload()/optionalResponseXML() directly; it now delegates to wrapped().visitAdditionalChildrenInGCThread(visitor), moving the unsynchronized member access into XMLHttpRequest where the lock lives.
• Source/WebCore/xml/XMLHttpRequest.cpp — Adds a new XMLHttpRequest::visitAdditionalChildrenInGCThread that takes Locker{m_gcLock} before adding opaque roots for m_upload and m_responseDocument. Every main-thread mutation of m_responseDocument (responseXML branches, clearResponseBuffers) and every access to m_upload (upload(), send, sendBytesData, createRequest, didSendData, dispatchErrorEvents, updateHasRelevantEventListener) is now wrapped in the same lock, snapshotting m_upload into a local RefPtr under the lock before any callout. Includes WebCoreOpaqueRootInlines.h and emits DEFINE_VISIT_ADDITIONAL_CHILDREN_IN_GC_THREAD(XMLHttpRequest).
• Source/WebCore/xml/XMLHttpRequest.h — Removes the now-unused optionalResponseXML()/optionalUpload() inline getters, declares the templated visitAdditionalChildrenInGCThread, adds Lock m_gcLock, and annotates both m_upload and m_responseDocument with WTF_GUARDED_BY_LOCK(m_gcLock) so the clang thread-safety analyzer enforces the discipline.

Background

Opaque roots / visitAdditionalChildrenInGCThread — WebKit’s binding layer marks C++ objects that must stay alive because a wrapped DOM object references them by adding them as ‘opaque roots’ during GC marking. This runs on a concurrent GC thread via the DEFINE_VISIT_ADDITIONAL_CHILDREN_IN_GC_THREAD machinery. Because it runs off the main thread, any member it reads must either be immutable or synchronized.

WTF_GUARDED_BY_LOCK — A clang thread-safety annotation used throughout WebCore/WTF. It statically requires that a given lock is held whenever the annotated field is accessed, turning a runtime race into a compile-time error. The patch applies it to both m_upload and m_responseDocument, which is how WebKit codifies the new invariant.

m_responseDocument lifecycle — responseXML() lazily parses the response into a Document and caches it in m_responseDocument, or sets it to nullptr for non-well-formed XML or the text/html-without-document-responseType case. clearResponseBuffers() resets it when a request is reset. These main-thread writes are the mutation side of the race.

RefPtr concurrency — WTF::RefPtr is not thread-safe for concurrent read/write of the same slot. Assigning nullptr to a RefPtr both decrements the referent’s refcount (possibly freeing it) and stores null; a concurrent reader on the GC thread can observe a torn or freed pointer, hence the need for a lock rather than merely atomic loads.

Vulnerability window

1. Baseline design — XMLHttpRequest exposes m_upload and m_responseDocument, both mutated on the main thread and both read by the GC visitor with no synchronization.
2. Trigger — Script issues an XHR, accesses responseXML (or aborts/reopens to hit clearResponseBuffers) while a concurrent GC marking cycle visits the JSXMLHttpRequest wrapper.
3. Race window — GC thread loads m_responseDocument.get(); main thread simultaneously assigns m_responseDocument = nullptr, dropping the last ref and freeing the Document.
4. UAF — GC thread calls addWebCoreOpaqueRoot on the freed Document, reading/marking dangling memory.
5. Fix — m_gcLock introduced; all readers/writers of both members serialize through it, and the GC visitor holds it while adding opaque roots.

Triggering

No test is included in the patch (this is a hardening/lifetime fix landed from a security branch). Trigger conceptually: create an XHR, set responseType so a responseXML Document is produced, then in a tight loop repeatedly read xhr.responseXML while re-opening/aborting the request (which runs clearResponseBuffers -> m_responseDocument = nullptr) and allocate heavily to keep the concurrent GC marking thread active, so the marker visits the wrapper exactly as the main thread nulls the RefPtr. Reproduction is inherently probabilistic and best observed under a TSAN/ASAN build.

Exploitation

1. Reachability — Fully script-reachable: XHR and its responseXML/upload members are standard web APIs, and concurrent GC marking is always on.
2. Race timing — Attacker maximizes the window by driving allocation to keep the marking thread busy and repeatedly toggling the document/upload state on the main thread; success is probabilistic per GC cycle.
3. Primitive — A freed Document is read and marked by the GC thread. This is honestly a race/UAF that most reliably manifests as a crash; turning it into a controlled use-after-free would require grooming the freed Document’s slot with attacker-controlled data and winning the race, which is not demonstrated here.

Detection & hunting

For defenders and SOC / detection engineers:

• TSAN data-race report —
• ASAN heap-use-after-free during GC marking —
• Crash clustering in XMLHttpRequest opaque-root visiting —

Audit directions

• Other visitAdditionalChildrenInGCThread implementors —
• Binding custom visitors —
• Lazily-initialized DOM members —
• Callout-under-lock hazards —