bafdda395e  [Site Isolation] Web Inspector: disable instrumentation for the old process on didCommitProvisionalPage

The bug at a glance

A UI-process lifetime bug in the Web Inspector’s Site-Isolation instrumentation: an IPC message-receiver registration on a WebProcessProxy could outlive the proxy itself because full-page cross-origin navigation (didCommitProvisionalPage) never paired the add with a remove, and cleanup relied on WebProcessProxy::processForIdentifier() which returns null after the proxy is destructed even though its receiver-map entry is not yet torn down. The observed symptom is a debug ASSERT(!m_messageReceiverMapCount) in ~MessageReceiver, but the commit explicitly calls it the same latent UAF pattern (using a destructed process’s receiver map). Medium/0.6: requires Web Inspector attached during a cross-origin navigation; not a remote-content primitive on its own.

ProxyingNetworkAgent registers an IPC receiver on each instrumented WebProcessProxy. Two gaps: didCommitProvisionalPage (full-page cross-origin nav) never called disableInstrumentationForProcess on the old process, so the add was unpaired; and the cleanup walks used processForIdentifier() to re-find the process to remove the receiver from, which returns null once the proxy is destructed – so removeMessageReceiver never runs, m_messageReceiverMapCount stays nonzero, and ~MessageReceiver asserts (a debug signal for a real dangling-receiver/UAF condition).

Root cause

ProxyingNetworkAgent proxies network instrumentation for inspected pages. enableInstrumentationForProcess(webProcess, pageID) records a (ProcessIdentifier, PageIdentifier) count in m_instrumentedProcessPageCounts and, on the first registration, calls webProcess.addMessageReceiver(…). Every add must be matched by a removeMessageReceiver on the same process, which decrements that receiver’s m_messageReceiverMapCount; ~MessageReceiver ASSERTs that the count is zero.

Gap (1): on a full-page cross-origin navigation the process swaps, and the hook that fires is WebPageInspectorController::didCommitProvisionalPage. Unlike didCommitProvisionalFrame (the iframe-swap hook), it did not touch the network agent, so it never called disableInstrumentationForProcess(oldProcess, oldWebPageID). The old process’s registration was left in m_instrumentedProcessPageCounts with its IPC receiver still installed – an unpaired add. The patch mirrors the frame path: if the network agent is enabled, disableInstrumentationForProcess for the old (process, pageID) and enableInstrumentationForProcess for the new (process, pageID).

Gap (2), the lifetime bug: the cleanup walks in removeAllRegisteredReceivers() and disable() iterated m_instrumentedProcessPageCounts by identifier and re-found the proxy via WebKit::WebProcessProxy::processForIdentifier(processID) in order to call removeMessageReceiver / send on it. But once a WebProcessProxy has been destructed, processForIdentifier() returns null – while its receiver-map entry has not yet been torn down (~AuxiliaryProcessProxy does not invalidate its receiver map). So removeMessageReceiver was skipped, m_messageReceiverMapCount stayed nonzero, and ~MessageReceiver fired ASSERT(!m_messageReceiverMapCount). The commit states ProxyingNetworkAgent has the identical pattern and the same latent UAF as the ProxyingPageAgent case: code holding a receiver registration against a process it does not keep alive.

The fix adds a parallel HashMap<ProcessIdentifier, Ref<WebProcessProxy>> m_pinnedInstrumentedProcesses. enableInstrumentationForProcess ensures a Ref pin for the process; disableInstrumentationForProcess drops the pin once that process has no remaining (process, pageID) registrations. The cleanup walks now look up the pinned Ref instead of calling processForIdentifier(), so the process (and its receiver map) is guaranteed alive when removeMessageReceiver / send run. This both silences the assert and removes the underlying use of a destructed process’s receiver map.

Key code

ProxyingNetworkAgent.cpp: use pinned Refs instead of nullable processForIdentifier()

    for (auto& [key, _] : std::exchange(m_instrumentedProcessPageCounts, { })) {
        auto [processID, pageID] = key;
        auto it = m_pinnedInstrumentedProcesses.find(processID);
        ASSERT(it != m_pinnedInstrumentedProcesses.end());
        if (it == m_pinnedInstrumentedProcesses.end())
            continue;
        Ref webProcess = it->value;
        webProcess->removeMessageReceiver(Messages::ProxyingNetworkAgent::messageReceiverName(), pageID);
    }
    m_pinnedInstrumentedProcesses.clear();

Patch walkthrough

• Source/WebKit/UIProcess/Inspector/WebPageInspectorController.cpp — didCommitProvisionalPage now handles the network agent symmetrically with the page agent: if m_networkAgent is enabled, call disableInstrumentationForProcess(*oldProcess, oldWebPageID) when there is an old process, then enableInstrumentationForProcess(newProcess, newWebPageID). This pairs the add/remove across full-page cross-origin navigation, which previously leaked the old process’s receiver registration.
• Source/WebKit/UIProcess/Inspector/Agents/ProxyingNetworkAgent.h — Adds HashMap<WebCore::ProcessIdentifier, Ref<WebKit::WebProcessProxy>> m_pinnedInstrumentedProcesses with a comment explaining it pins each instrumented process alive while a receiver registration is held, so the process cannot be destructed before ~ProxyingNetworkAgent, which otherwise leaves m_messageReceiverMapCount nonzero and asserts in ~MessageReceiver.
• Source/WebKit/UIProcess/Inspector/Agents/ProxyingNetworkAgent.cpp — enableInstrumentationForProcess ensure()s a Ref pin in m_pinnedInstrumentedProcesses on first registration. disableInstrumentationForProcess drops the pin once no (process,pageID) entries remain for that process. removeAllRegisteredReceivers() and disable() now use the pinned Ref (with an ASSERT that it exists) instead of WebProcessProxy::processForIdentifier(), guaranteeing the process is alive for removeMessageReceiver/send; removeAllRegisteredReceivers() clears the pin map at the end.

Background

IPC MessageReceiver map — AuxiliaryProcessProxy keeps a map of registered message receivers keyed by name and destination id. addMessageReceiver increments a receiver’s m_messageReceiverMapCount and removeMessageReceiver decrements it; ~MessageReceiver ASSERTs the count is zero, catching receivers destroyed while still registered.

WebProcessProxy::processForIdentifier() — Global lookup from a ProcessIdentifier to a live WebProcessProxy. It returns null once the proxy has been destructed – but ~AuxiliaryProcessProxy does not invalidate the receiver map, so a stale registration can outlive the ability to find the process to remove it.

didCommitProvisionalPage vs didCommitProvisionalFrame — Process-swap hooks in WebPageInspectorController: the Page hook fires on full-page cross-origin navigation, the Frame hook on iframe swaps. Instrumentation must be disabled on the old process and enabled on the new one in both; only the Frame path did so for the network agent.

Pinning with Ref<WebProcessProxy> — Holding a Ref keeps the process object alive. ProxyingPageAgent already used this proven pattern; the fix adds the same m_pinnedInstrumentedProcesses map to ProxyingNetworkAgent so a pinned process is guaranteed live wherever a receiver registration is outstanding.

Vulnerability window

1. Prior fix — 313207@main introduced the assertion exposure; 313741@main made disable() reach stale processes via the registration map, fixing one half of the receiver leak.
2. Residual leak — didCommitProvisionalPage still did not disable network instrumentation on the old process for full-page cross-origin nav, leaving m_instrumentedProcessPageCounts and the IPC receiver referencing the swapped-out process.
3. Latent UAF — Cleanup used processForIdentifier(), which returns null after the proxy is destructed while the receiver map entry persists, so removeMessageReceiver was skipped and ~MessageReceiver’s ASSERT(!m_messageReceiverMapCount) flaked (webkit.org/b/315312, rdar://177178776).
4. Fix — didCommitProvisionalPage pairs add/remove for the network agent, and both agents pin the WebProcessProxy via m_pinnedInstrumentedProcesses so cleanup uses a guaranteed-live Ref (canonical 314195@main).
5. Verification — The previously-skipped Debug expectations for http/tests/site-isolation/inspector/target/target.html and target-cross-origin-page-navigation.html are re-enabled.

Exploitation

1. Attach + navigate — With the inspector’s network instrumentation on under Site Isolation, drive a full-page cross-origin navigation so didCommitProvisionalPage swaps processes and (pre-fix) leaves the old process’s receiver registration unpaired.
2. Destruct the proxy — Let the swapped-out WebProcessProxy be destroyed. processForIdentifier() now returns null so no removeMessageReceiver runs and the receiver map entry dangles; the receiver’s destructor references a map on an already-freed process.
3. Assert / UAF — In Debug this surfaces as ASSERT(!m_messageReceiverMapCount); the commit identifies the same pattern as a latent use-after-free of the destructed process’s receiver map. Exploitability requires an attached inspector, so it is a limited local/tooling surface. INFERRED beyond the assert.

Detection & hunting

For defenders and SOC / detection engineers:

• ASSERT(!m_messageReceiverMapCount) —
• processForIdentifier() returning null in cleanup —

Audit directions

• add/removeMessageReceiver pairing across process swaps —
• processForIdentifier() in cleanup paths —
• Other Proxying agents* —