[5] NetworkProcess null-deref on a BroadcastChannel message with a null name

A null channel name crashed the NetworkProcess while hashing the key — faulting before the hash table's own validator ever got to run.

Severity: Low | Component: WebKit NetworkProcess IPC surface | e85a1ea

Rated Low because the diff fixes a reliably reachable null-StringImpl dereference in the NetworkProcess driven by a forged IPC message; the faulting address carries no attacker-controlled data, so the observable effect is a controlled crash (availability) with no path to memory corruption, and reaching it already requires a compromised WebProcess.

NetworkBroadcastChannelRegistry uses the IPC-supplied channel name as a HashMap<String, ...> key in registerChannel() (via ensure()) and in unregisterChannel() / postMessage() (via find()). A compromised or malformed WebProcess could send a null String for the name; looking up a null String key dereferences a null StringImpl while hashing the key (StringHash::hash() calls key.impl()->hash()), crashing the network process before HashTable::validateKey() ever runs. The fix rejects a null name with a MESSAGE_CHECK in all three endpoints, matching the existing origin validation.

Source/WebKit/NetworkProcess/NetworkBroadcastChannelRegistry.cpp

 void NetworkBroadcastChannelRegistry::registerChannel(IPC::Connection& connection, const WebCore::ClientOrigin& origin, const String& name)
 {
     MESSAGE_CHECK(isValidClientOrigin(origin), connection);
+    MESSAGE_CHECK(!name.isNull(), connection);
 
     auto& channelsForOrigin = m_broadcastChannels.ensure(origin, [] { return NameToConnectionIdentifiersMap { }; }).iterator->value;
     auto& connectionIdentifiersForName = channelsForOrigin.ensure(name, [] { return Vector<IPC::Connection::UniqueID> { }; }).iterator->value;
 ...
 void NetworkBroadcastChannelRegistry::unregisterChannel(...) {
     MESSAGE_CHECK(isValidClientOrigin(origin), connection);
+    MESSAGE_CHECK(!name.isNull(), connection);
 ...
 void NetworkBroadcastChannelRegistry::postMessage(...) {
     MESSAGE_CHECK_COMPLETION(isValidClientOrigin(origin), connection, completionHandler());
+    MESSAGE_CHECK_COMPLETION(!name.isNull(), connection, completionHandler());

LayoutTests/ipc/coreipc.js

             case 'String':
+                if (argument === null)
+                    return {value: null, type: 'String'};

Patch Details

registerChannel() and unregisterChannel() gain MESSAGE_CHECK(!name.isNull(), connection), and postMessage() gains MESSAGE_CHECK_COMPLETION(!name.isNull(), connection, completionHandler()), each placed immediately after the existing isValidClientOrigin(origin) check and before the String name is used as a HashMap key. The remaining diff is collateral: a new layout test driving the bug through the IPC testing API, and a one-line change to coreipc.js's ArgumentSerializer to allow serializing a null value for a String argument.

Missing null-validation of attacker-controlled IPC input before it is used as a hash-map key that dereferences its backing StringImpl during hashing.

Background

BroadcastChannel is a Web API letting same-origin browsing contexts message each other; the NetworkProcess maintains the central registry so messages route across WebProcesses. MESSAGE_CHECK is a WebKit IPC macro that validates an assertion on an incoming message and, on failure, terminates the offending connection rather than continuing. A WTF String wraps a refcounted StringImpl; a null String has a null impl(), and StringHash::hash() computes a key's hash by calling key.impl()->hash(). HashMap insertion/lookup hashes the key, and only later runs HashTable::validateKey(). The IPCTestingAPI is a test-only facility that lets a layout test synthesize raw IPC messages to a target process, used here to reproduce a message a compromised WebProcess could send.

Analysis

This is a null pointer dereference from unvalidated IPC input. Before the fix, NetworkBroadcastChannelRegistry trusted the IPC-supplied channel name and used it directly as a HashMap<String, ...> key without validating it was non-null. The endpoints validated the ClientOrigin via isValidClientOrigin but applied no equivalent check to name. When a null String is used as a key, the hash table must compute the key's hash before it can reach HashTable::validateKey(): StringHash::hash() calls key.impl()->hash(), and for a null String impl() returns nullptr, dereferencing a null StringImpl.

The mechanism is that m_broadcastChannels.ensure(origin, ...) succeeds, then channelsForOrigin.ensure(name, ...) (register) or .find(name) (unregister/postMessage) hashes the null key and faults during the hash computation. From a compromised WebProcess (or via the IPCTestingAPI in the test), an attacker sends a RegisterChannel/UnregisterChannel/PostMessage IPC with a valid ClientOrigin and name : null; the NetworkProcess passes the origin check, hashes the null name, and crashes. There is no attacker-controlled data at the faulting address, so this does not escalate beyond a crash without an unrelated primitive.

This vulnerability weakens the WebProcess→NetworkProcess IPC trust boundary's robustness against malformed input. The NetworkProcess assumed (but did not enforce) that a registered BroadcastChannel name is a non-null String; a compromised WebProcess forging a null name violates that. A successful trigger crashes the shared NetworkProcess, terminating networking for all browsing contexts — a denial of service affecting availability. It does not cross a memory-safety or origin boundary.

The faulting site is subtle because the crash happens during hash computation (StringImpl::hash) before HashTable::validateKey() runs — so any defensive validation living inside the hash table is bypassed. Whenever attacker-controlled Strings reach a HashMap key, null/empty validation must happen at the IPC boundary, not be delegated to the container.

Note: The precise WTF hashing mechanism (that StringHash::hash() faults on a null StringImpl before validateKey()) and the cross-process routing of BroadcastChannel through the NetworkProcess rely on WTF/WebKit internals not fully visible in the diff. The missing null-check and its placement before the hash-keyed lookup are directly supported by the patch.

Audit directions

IPC handlers that validate one field of a struct but use a sibling field as a HashMap key without null-checking it. Audit other NetworkProcess registries that key maps on IPC-supplied Strings (ServiceWorker, storage, cache name maps) for handlers that check origin but not the name/key. Grep Source/WebKit/NetworkProcess for .ensure( and .find( calls whose key is an IPC-passed const String& and confirm a preceding MESSAGE_CHECK(!name.isNull(), ...).
A null String/StringImpl used as a HashMap key crashes during StringHash::hash() before HashTable::validateKey() runs. Verify that container-level key validation is not relied upon as the security boundary for IPC input across WebKit; search for validateKey assumptions and confirm IPC entry points reject null keys themselves.
Symmetry of validation across related endpoints. When one IPC method (e.g. registerChannel) gains a guard, check that every method consuming the same key (unregisterChannel, postMessage, and any removeConnection-style cleanup paths) enforces the identical invariant, since maps populated under one guard may be queried under another.