8e3ad95fd1  Crash in HistoryController::updateForCommit() when calling navigation.reload() during pageswap event handler

The bug at a glance

OBSERVED: Navigation::reload() gains a frame()->loader().isDispatchingPageSwapEvent() guard (matching the one already in Navigation::navigate()), so a navigation.reload() invoked during pageswap dispatch is rejected with InvalidStateError. INFERRED per the commit message: a reload transitioning to committed dispatches pageswap; a navigation.reload() inside that handler runs a synchronous policy check that clears the provisional DocumentLoader, and after the event returns HistoryController::updateForCommit() dereferences the now-null FrameLoader::provisionalDocumentLoader(). Medium: a reentrancy-induced null-deref / loader-state UAF crash in the navigation pipeline, script-triggerable but a crash rather than demonstrated corruption.

This is reentrancy into the loader during a fragile window. The pageswap event fires mid-commit, while the loader is between provisional and committed state. Re-entering the navigation machinery via navigation.reload() from that handler runs a synchronous policy decision that tears down the provisional DocumentLoader that the outer commit is still about to use. When control returns to updateForCommit() the pointer it relies on is gone. The fix simply refuses the reentrant reload, extending an identical guard previously added for navigate().

Root cause

The Navigation API (navigation.reload(), navigation.navigate()) drives same-document and cross-document navigations. A cross-document reload that transitions its DocumentLoader to committed dispatches a pageswap event so the page can react as the old document is swapped out. During this dispatch the FrameLoader is in a delicate intermediate state: the provisional DocumentLoader has been created and the commit sequence, culminating in HistoryController::updateForCommit(), is in progress and still expects FrameLoader::provisionalDocumentLoader() to be non-null.

The defect is reentrancy from the pageswap handler. If script inside onpageswap calls navigation.reload(), that call performs a synchronous policy check (the navigation policy decision). As a side effect of starting a new navigation, that synchronous path clears the in-flight provisional DocumentLoader. Control then unwinds back out of the event dispatch into the original commit sequence, and HistoryController::updateForCommit() dereferences FrameLoader::provisionalDocumentLoader(), which is now null – crashing the process.

The pre-patch guard in Navigation::reload() only checked !document->isFullyActive() || document->unloadCounter(). Notably Navigation::navigate() had already been hardened for the identical scenario in bug 303364 by adding an isDispatchingPageSwapEvent() check, but reload() was missed. The fix brings reload() to parity: it restructures the guard to take a RefPtr document = window->document() and adds frame()->loader().isDispatchingPageSwapEvent() to the disqualifying conditions, so a reload attempted while the pageswap event is being dispatched returns createErrorResult(…, ExceptionCode::InvalidStateError, “Invalid state”). This blocks the reentrant teardown of the provisional loader entirely. OBSERVED: the added test registers onpageswap, calls navigation.reload() inside it, and asserts both the committed and finished promises reject (rather than crashing).

Key code

Reject navigation.reload() during pageswap dispatch (Navigation::reload)

    RefPtr window = this->window();
-    if (!protect(window->document())->isFullyActive() || window->document()->unloadCounter())
+    if (RefPtr document = window->document(); !document->isFullyActive() || frame()->loader().isDispatchingPageSwapEvent() || document->unloadCounter())
        return createErrorResult(WTF::move(committed), WTF::move(finished), ExceptionCode::InvalidStateError, "Invalid state"_s);

Patch walkthrough

• Source/WebCore/page/Navigation.cpp — In Navigation::reload(), the early-return guard was changed from if (!protect(window->document())->isFullyActive() || window->document()->unloadCounter()) to if (RefPtr document = window->document(); !document->isFullyActive() || frame()->loader().isDispatchingPageSwapEvent() || document->unloadCounter()). The new isDispatchingPageSwapEvent() clause rejects a reload issued while a pageswap event is being dispatched, preventing the synchronous policy check from clearing the provisional DocumentLoader that HistoryController::updateForCommit() will later dereference. This mirrors the guard already present in Navigation::navigate() from bug 303364.
• LayoutTests/fast/loader/reload-on-pageswap-crash.html — Reproduces the crash: triggers a navigation.reload(), and in the onpageswap handler calls navigation.reload() again, asserting both its committed and finished promises are rejected. ‘PASS if no crash.’

Background

pageswap event — Fired as a document is being swapped out during a cross-document navigation that has reached the committed transition. Handlers run mid-commit, while the loader is between provisional and committed state.

provisional DocumentLoader — The DocumentLoader for an in-flight navigation before it is committed. FrameLoader::provisionalDocumentLoader() returns it; the commit sequence (updateForCommit) assumes it stays valid until commit completes.

HistoryController::updateForCommit() — Part of the commit sequence that updates history state; it dereferences provisionalDocumentLoader(). If a reentrant navigation cleared that loader, it reads a null pointer and crashes.

isDispatchingPageSwapEvent() — FrameLoader flag indicating a pageswap event is currently being dispatched. Navigation::navigate() already consulted it (bug 303364); this patch adds the same consultation to Navigation::reload().

Vulnerability window

1. Prior fix — Bug 303364 added an isDispatchingPageSwapEvent() guard to Navigation::navigate() to stop reentrant navigation during pageswap dispatch.
2. Gap — Navigation::reload() was not given the equivalent guard, leaving the reload path reentrant.
3. Crash — A navigation.reload() during a pageswap handler synchronously cleared the provisional DocumentLoader; the outer commit’s updateForCommit() then dereferenced the null loader.
4. Discovery — bug 309782 / rdar://167842846 reported the updateForCommit() crash.
5. Fix — reload() guard extended with isDispatchingPageSwapEvent(); reentrant reloads reject with InvalidStateError. Test asserts no crash.

Proof of concept

VERBATIM from LayoutTests/fast/loader/reload-on-pageswap-crash.html. A first navigation.reload() drives a reload; the onpageswap handler installed before the commit calls navigation.reload() again, reentering the loader while the provisional DocumentLoader is live. Pre-patch this cleared that loader and crashed in HistoryController::updateForCommit(); post-patch both promises reject with InvalidStateError and no crash occurs.

(async () => {
  if (sessionStorage.testCompleted) {
    delete sessionStorage.testCompleted;
    testRunner?.notifyDone();
  } else {
    sessionStorage.testCompleted = true;
    await navigation.reload();
  }

  let audio = document.createElement('audio');
  audio.src = '/resources/foo.mp4';
  await navigator.locks.query();
  onpageswap = async (e) => {
    let { committed, finished } = navigation.reload();
    let committedRejected = await committed.then(() => false, () => true);
    let finishedRejected = await finished.then(() => false, () => true);
    if (!committedRejected)
      document.write("FAIL: committed promise should have been rejected.");
    if (!finishedRejected)
      document.write("FAIL: finished promise should have been rejected.");
  };
  await cookieStore.get("bar");
})();

Exploitation

1. Arm pageswap — Script registers an onpageswap handler and initiates a cross-document navigation.reload() so pageswap will fire during commit.
2. Reenter — Inside the handler, call navigation.reload() again; its synchronous policy check clears the provisional DocumentLoader out from under the in-progress commit.
3. Null-deref crash — When commit resumes, HistoryController::updateForCommit() dereferences the now-null provisionalDocumentLoader(), crashing the process – a reliable renderer DoS; deeper loader-state corruption from reentrancy is the theoretical upper bound.

Detection & hunting

For defenders and SOC / detection engineers:

• Null provisionalDocumentLoader in updateForCommit —
• reload() invoked during pageswap —

Audit directions

• Navigation API reentrancy —
• Commit-sequence loader lifetime —
• Synchronous policy checks —