Invalid IPC WebResourceLoader_WillSendRequestReply crash + cookie-leak merge-back

5243996

In WebKit's multi-process model the NetworkProcess handles actual network I/O while the WebContent process — which can be attacker-controlled via a renderer exploit — only proposes requests. continueWillSendRedirectedRequest is the one callsite where the web process gets to modify a request mid-redirect before it is dispatched to CFNetwork, including the firstPartyForCookies field that determines which origin's cookies get attached. MESSAGE_CHECK_COMPLETION_BASE is WebKit's mechanism for validating IPC-supplied data and killing the sending process if it violates protocol invariants.

Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

 Ref connection = protectedThis->m_connection;
+if (!connection->connection().isValid())
+    return completionHandler({ });
+
+if (newRequest.isNull())
+    return completionHandler({ });
+
 if (newRequest.firstPartyForCookies() != firstPartyForCookiesFromRedirectRequest) {
     auto allowCookieAccess = connection->networkProcess().allowsFirstPartyForCookies(connection->webProcessIdentifier(), newRequest.firstPartyForCookies());
     MESSAGE_CHECK_COMPLETION_BASE(allowCookieAccess == NetworkProcess::AllowCookieAccess::Allow, connection->connection(), completionHandler({ }));
 }

This adds validity/null checks before the message check in continueWillSendRedirectedRequest, and reorders the firstPartyForCookies check to run only inside the mismatch branch. It is a merge-back of a security fix (bug 313866) that closes a cross-origin cookie leak, plus a follow-up fix for crashes caused by the original fix. The tricky case being handled is distinguishing an actually malicious modification from a benign teardown: a keepalive request whose IPC connection is torn down mid-flight previously produced a default-constructed (null) request that could spuriously trip the check and kill a legitimate process.

Significance

Closes a path where a compromised WebContent process could rewrite a redirect's firstPartyForCookies to obtain cookies from an origin it shouldn't reach, while stopping spurious process kills when the web process exits mid-redirect. The combination matters because the original cookie-leak fix hardened the trust boundary but widened a crash surface on the keepalive/teardown race; this merge-back keeps the security property and removes the availability regression.

Audit directions

This is a direct case study in cross-process trust boundaries: the WebContent process is untrusted, and every field it can influence in a redirected request must be validated in the NetworkProcess before use. The forward-facing pattern hunt is other ResourceRequest fields modifiable at this same callsite — headers, credentials policy, referrer — which deserve the same "is this value in the web process's allowed set" check that firstPartyForCookies now gets; audit whether each is validated or trusted. The teardown-race shape generalizes too: any IPC completion handler that treats connection-teardown-produced default-constructed values as legitimate input can trip an adjacent MESSAGE_CHECK — audit the peer completion handlers in NetworkResourceLoader and the analogous GPUProcess reply paths for the same pattern, where a null/invalid-connection early-return is missing ahead of a protocol check. Match tell: a MESSAGE_CHECK* keyed on a decoded IPC value with no preceding isValid()/isNull() guard on the connection or the payload.