[4] DocumentThreadableLoader null-WeakPtr dereference on m_document

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.

Audit directions