[3] DocumentThreadableLoader dereferences a dead document WeakPtr

MediumWebCore loaderUAF

8384754

Medium. Web content picks the timing here: start a preflighted cross-origin fetch, detach the frame, and the completion callback walks into a RELEASE_ASSERT that kills the renderer. The assert is also what caps it — it aborts before any pointer arithmetic on null storage, so there is no memory-safety upside for an attacker.

Asynchronous loads in WebKit outlive the DOM objects that started them. A loader kicked off by fetch() or XMLHttpRequest stays owned by the network and CORS state machine until its request completes, and it deliberately does not keep its originating document alive — holding a strong reference to the document from a long-lived loader would leak the entire frame. The loader instead stores a weak reference and is expected to check whether the document is still there before touching it; the invariant is that every callback arriving after teardown handles the absence gracefully.

The angle: a page can start a cross-origin fetch that needs preflight, tear down the frame that issued it, and reliably abort the WebContent process when the preflight result comes back.

This patch adds liveliness checks for dereferencing a WeakPtr in WebCore::DocumentThreadableLoader. Previously the m_document WeakPtr was dereferenced by calling the document() or protectedDocument() member functions.

Since it's possible for the WeakPtr m_document to be null, we should add checks before dereferencing it to avoid hitting a RELEASE_ASSERT in WeakPtr's * operator. To ensure that m_document is kept alive after performing the null check, we convert it to a RefPtr.

Source/WebCore/loader/DocumentThreadableLoader.h

Source/WebCore/loader/DocumentThreadableLoader.cpp

Source/WebCore/loader/CrossOriginPreflightChecker.cpp

Patch Details

The change rewrites the loader's document accessor to make the null case representable, converts every access site to a null-checked local strong reference, and drops the preflight checker from the safer-CPP exemption list.

DocumentThreadableLoader::document() changes from returning Document& (via *m_document) to returning a raw Document*. Every call site in DocumentThreadableLoader.cpp and CrossOriginPreflightChecker.cpp now captures m_document into a local RefPtr, null-checks it, and only then calls methods on it — shouldSetHTTPHeadersToKeep, makeCrossOriginAccessRequest, cancel, didReceiveResponse, didFail, preflightFailure, loadRequest, securityOrigin, contentSecurityPolicy, crossOriginEmbedderPolicy, and logErrorAndFail on the loader side, plus validatePreflightResponse, notifyFinished, startPreflight, and doPreflight in the preflight checker. Third, loader/CrossOriginPreflightChecker.cpp is removed from Source/WebCore/SaferCPPExpectations/UncheckedCallArgsCheckerExpectations, indicating the file now passes WebKit's safer-CPP unchecked-argument static check.

Stale WeakPtr dereference whose RELEASE_ASSERT-on-null was reachable from web-content-driven loader callbacks.

Background

Where this lives. DocumentThreadableLoader is the WebCore class that performs asynchronous and synchronous loads on behalf of a Document — it is the backend for fetch(), XMLHttpRequest, EventSource, and similar APIs, and it drives CORS preflight through CrossOriginPreflightChecker.

Loader lifetime vs. document lifetime. The loader is RefCounted and can outlive its originating Document. When a frame is detached or a document is replaced, the document is destroyed but in-flight loaders — still owned by the network/CORS state machine — keep running and eventually deliver completion or error callbacks. The loader stores its document as WeakPtr<Document, WeakPtrImplWithEventTargetData> m_document, precisely so that it does not extend the document's lifetime.

WeakPtr and RefPtr. WeakPtr<T> in WebKit is a non-owning smart pointer that goes null when the referent is destroyed; calling operator* or operator-> on a null WeakPtr triggers a RELEASE_ASSERT — always on, including release builds — which aborts the process. RefPtr<T> is a reference-counted owning smart pointer; assigning a WeakPtr into a RefPtr either captures a strong reference, extending lifetime for the enclosing scope, or evaluates to null if the referent has already been destroyed.

Analysis

The bug class is a null-WeakPtr dereference that escalates into a forced process abort. Before the fix, DocumentThreadableLoader::document() returned a Document& produced by dereferencing m_document with operator*, and many code paths in the file dereferenced the member directly.

  Web content                 Loader                        Document
  ───────────                 ──────                        ────────
  fetch(cross-origin,
        custom headers)  ──►  preflight in flight
                                                            live
  iframe.remove()  ─────────────────────────────────────►   destroyed
                                                            m_document → null
  (preflight response)  ──►   preflightFailure()
                              m_document->frame()
                                └─ WeakPtr::operator*
                                     RELEASE_ASSERT → abort

Because DocumentThreadableLoader legitimately outlives its Document in several scenarios — asynchronous CORS preflight in flight, redirect callbacks, error reporting after frame detach, service-worker-mediated paths — any of those unchecked dereferences could trip the release-assert and terminate the WebContent process. The claim that WeakPtr::operator* carries that RELEASE_ASSERT is the commit's own stated rationale for the fix and is relayed from the commit message; the WTF implementation is not part of the supplied context.

The trigger from web content is straightforward: open a cross-origin fetch() that requires preflight — a request with custom headers or a non-simple method — inside an iframe or window the attacker controls, then synchronously detach the document by removing the iframe, navigating the frame, or closing the window while the preflight is in flight. When preflight completion, redirect, or failure callbacks arrive, the corresponding loader or preflight-checker method dereferences the now-null m_document. Pre-fix call sites preflightFailure (m_document->frame()), cancel (m_document->identifier()), loadRequest (m_document->frame()), and validatePreflightResponse (loader.document().frame()) are all reachable along these paths.

The resulting primitive is a reliable, web-content-triggered crash of the WebContent process, and nothing more. The RELEASE_ASSERT in WeakPtr::operator* deliberately aborts before any pointer arithmetic on the null storage occurs, so this does not yield a read/write or type-confusion primitive, and a renderer crash does not cross the sandbox boundary into the GPU, Networking, or UI process.

The discovery angle is visible in the diff itself: removing loader/CrossOriginPreflightChecker.cpp from UncheckedCallArgsCheckerExpectations means the file was on a known-bad list for WebKit's in-tree safer-CPP static checker, and passing WeakPtr-derived references as call arguments without a null check is exactly what an unchecked-call-args checker flags. Crash telemetry from RELEASE_ASSERT aborts inside WeakPtr::operator* is an equally plausible trigger.

This vulnerability weakens WebContent process availability. The HTML loading lifecycle assumes DocumentThreadableLoader callbacks see a live Document; before the fix that invariant was enforced by a RELEASE_ASSERT rather than a graceful null check, so any path where the document detached between request initiation and callback terminated the renderer.

Insight: this is a recurring shape in WebKit. Components that legitimately outlive their owning Document cache it as a WeakPtr to avoid lifetime extension, then dereference it with operator*/operator-> instead of converting to RefPtr and null-checking. The RELEASE_ASSERT converts these latent UAF-shaped bugs into deterministic process aborts — a defence-in-depth win against memory corruption that leaves behind a large surface of web-content-reachable crashes. The header-level half of this fix is the more durable mitigation: changing document() to return Document* pushes the null case into the type system and forces every caller to handle it.

Audit directions

• Long-lived loader/observer objects storing WeakPtr<Document> and dereferencing it on async callbacks. The RELEASE_ASSERT in WeakPtr::operator* turns every such unchecked dereference into a renderer-killable DoS reachable from web content. Grep Source/WebCore for m_document->, m_frame->, m_window-> where the surrounding member is a WeakPtr, and verify a RefPtr plus null-check precedes the dereference; start with peer loaders like WorkerThreadableLoader, PingLoader, BeaconLoader, and EventSource.
• Accessor methods that return a reference by dereferencing an internal WeakPtr. The reference return type hides the null case from callers and propagates RELEASE_ASSERT risk through every call site. Audit class headers for declarations of the form T& foo() { return *m_weakMember; } and convert them to T* foo() { return m_weakMember.get(); } or similar. Start with DocumentThreadableLoader.h-style accessors across Source/WebCore/loader/ and Source/WebCore/page/. In review, a one-line accessor returning *m_someWeakMember is the tell — the dereference is invisible at every call site.
• The safer-CPP exemption lists as a bug inventory. UncheckedCallArgsCheckerExpectations and its siblings in Source/WebCore/SaferCPPExpectations/ list files explicitly grandfathered out of WebKit's static checks; each entry is a known-bad file. Examine each remaining entry — particularly under loader/, dom/, and html/ — and reproduce the same WeakPtr-to-RefPtr conversion treatment performed here; many will harbour the same class of crash bug.
• CORS-preflight and redirect callback paths that assume document liveness. The synchronous request starts from a live document, but completion arrives arbitrarily later. Trace CrossOriginPreflightChecker, SubresourceLoader, and CachedResourceClient callbacks (notifyFinished, redirectReceived, responseReceived) and verify each null-checks the originating context before touching it. Start with Source/WebCore/loader/cache/CachedResourceClient.h implementers.