[4] DocumentThreadableLoader null-WeakPtr dereference on m_document

`DocumentThreadableLoader` deref'd its `WeakPtr<Document>` directly — async preflight callbacks after iframe detach became a renderer kill switch.

Severity: Medium | Component: WebCore loader — DocumentThreadableLoader / CrossOriginPreflightChecker | 8384754

Rated Medium because the diff fixes a null WeakPtr dereference on m_document reachable from CORS preflight / redirect / failure callbacks after the originating document has been detached. The RELEASE_ASSERT in WeakPtr::operator* produces a deterministic, web-content-triggered renderer abort; no read/write primitive is exposed because the assert fires before any pointer arithmetic on the null storage occurs.

This patch adds liveness checks before dereferencing m_document in DocumentThreadableLoader and CrossOriginPreflightChecker. Previously the WeakPtr was dereferenced through document() / protectedDocument(). Because m_document can be null, the existing pattern relied on WeakPtr::operator*'s RELEASE_ASSERT, which aborts the process on null. The fix converts each access site to a local RefPtr (extending the document's lifetime across the call window) preceded by an explicit null check, and changes document() to return Document* so the null case is visible in the type system. The header file is also removed from UncheckedCallArgsCheckerExpectations, indicating it now passes WebKit's safer-CPP unchecked-arg static check.

Source/WebCore/loader/DocumentThreadableLoader.h

Source/WebCore/loader/DocumentThreadableLoader.cpp

Source/WebCore/loader/CrossOriginPreflightChecker.cpp

Patch Details

The fix touches every site in DocumentThreadableLoader.cpp and CrossOriginPreflightChecker.cpp that previously dereferenced m_document: shouldSetHTTPHeadersToKeep, makeCrossOriginAccessRequest, cancel, didReceiveResponse, didFail, preflightFailure, loadRequest, securityOrigin, contentSecurityPolicy, crossOriginEmbedderPolicy, logErrorAndFail, plus validatePreflightResponse, notifyFinished, startPreflight, and doPreflight in the preflight checker. The accessor signature change (Document& document() → Document* document()) propagates the null case into the type system at every call site. No business logic is restructured; the fix is uniformly "capture into RefPtr, null-check, only then dereference".

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

Background

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, even in 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 scope) or evaluates to null if the referent has already been destroyed.

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 via CrossOriginPreflightChecker. 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 it does not extend the document's lifetime.

Analysis

Pre-fix, DocumentThreadableLoader::document() returned a Document& produced by dereferencing m_document with operator*. Because the loader can outlive its document in several scenarios — asynchronous CORS preflight in flight, redirect callbacks, error reporting after frame detach, service-worker-mediated paths — many code paths in this file dereferenced m_document without first checking liveness. Any of those paths could trip the RELEASE_ASSERT in WeakPtr::operator* and crash the WebContent process.

This is a recurring class in WebKit: components that legitimately outlive their owning Document cache it as a WeakPtr to avoid lifetime extension, but then dereference it with operator* / operator-> instead of converting to RefPtr and null-checking. The RELEASE_ASSERT in WeakPtr::operator* converts these latent UAF-shaped bugs into deterministic process aborts — that is a defense-in-depth win against memory corruption, but it leaves a large surface of web-content-reachable crashes.

From web content the path is direct: open a cross-origin fetch() that requires CORS preflight (e.g., a request with custom headers or a non-simple method) inside an iframe or window the attacker controls, then synchronously detach the document — remove the iframe, navigate the frame, or close the window — while the preflight is in flight. When preflight completion, redirect, or failure callbacks arrive, the corresponding DocumentThreadableLoader or CrossOriginPreflightChecker method dereferences the now-null m_document WeakPtr. Pre-fix call sites such as preflightFailure (m_document->frame()), cancel (m_document->identifier()), loadRequest (m_document->frame()), and validatePreflightResponse (loader.document().frame()) are all reachable along these paths.

The header-level fix here (changing document() to return Document*) is the more durable mitigation because it pushes the null case into the type system and forces every caller to handle it; the same pattern would help in other long-lived loader/observer classes that hold WeakPtr<Document>. The removal from UncheckedCallArgsCheckerExpectations indicates WebKit has a static checker actively flagging this pattern.

This vulnerability weakens WebContent process availability. The HTML loading lifecycle assumes that DocumentThreadableLoader callbacks see a live Document; pre-fix, that invariant was enforced by a RELEASE_ASSERT rather than a graceful null check, so any code path where the document detached between request initiation and callback would terminate the renderer. An attacker controlling a page that issues cross-origin fetches and tears down the originating document before the loader's callbacks complete could trigger a reliable, web-content-reachable crash of the WebContent process — a denial-of-service primitive against the renderer, not a memory-safety primitive.

Audit directions

• Long-lived loader/observer objects that store WeakPtr<Document> (or WeakPtr<Frame>, WeakPtr<LocalDOMWindow>) and dereference it on async callbacks 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 + null-check precedes the dereference. Start with peer loaders like WorkerThreadableLoader, PingLoader, BeaconLoader, and EventSource.

• Accessor methods that return a reference (T&) by dereferencing an internal WeakPtr. The reference 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(); }. Start with DocumentThreadableLoader.h-style accessors across Source/WebCore/loader/ and Source/WebCore/page/.

• UncheckedCallArgsCheckerExpectations and sibling *Expectations files in Source/WebCore/SaferCPPExpectations/. 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 assuming document liveness. Trace CrossOriginPreflightChecker, SubresourceLoader, and CachedResourceClient callbacks (notifyFinished, redirectReceived, responseReceived) and verify each callback null-checks the originating context before touching it. Start with Source/WebCore/loader/cache/CachedResourceClient.h implementers.

Note: Several claims (WeakPtr::operator*'s RELEASE_ASSERT behavior, the specific scenarios in which DocumentThreadableLoader outlives its Document, and the iframe-detach-during-preflight reachability sequence) are inferred from the patch's mitigation pattern and standard WTF idiom rather than visible in the diff. The fix mechanism — WeakPtr to RefPtr plus null check across every call site — is directly supported by the patch.