We should wait until we get a safe browsing response before proceeding with downloads

MediumWebKit UIProcess navigation policyLogicError

CVE: CVE-2026-28971 · Safari 26.5 · Released May 13, 2026 Impact: A malicious iframe may use another website's download settings Apple's description: The issue was addressed with improved UI handling. Credit: Khiem Tran

4b574bf | Bugzilla 311288

Medium — no memory corruption, just a reputation check that answers "no warning" when it means "no answer yet". The escalation condition is entirely under the server's control: outlast a ~250 ms timeout and a flagged file lands on disk with the interstitial never shown.

Browsers that consult a remote reputation service before committing to a navigation have to decide what happens when the service is slow, and WebKit's answer is to proceed and apologize later — the load continues, and if the verdict eventually says "malicious", a full-screen red interstitial is painted over the page that already loaded. That retroactive cover is what makes the timeout safe. It works because a page load is still there to be covered; PolicyAction::Download is the one policy outcome that ends the navigation, hands the response to the download machinery, and leaves nothing behind to paint on.

The angle: A page — or an iframe inside someone else's page — whose server response outruns the reputation lookup can have a flagged file written to the user's download folder with no fraudulent-website warning ever appearing.

Source/WebKit/UIProcess/API/APINavigation.cpp

Source/WebKit/UIProcess/API/APINavigation.h

Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm

Source/WebKit/UIProcess/WebPageProxy.cpp

Patch Details

The change has three parts: a small piece of continuation machinery on API::Navigation, one fire site in the Cocoa lookup completion block, and two structurally identical deferral branches in the two policy decision functions.

API::Navigation gains a Vector<Function<void()>> m_safeBrowsingCheckCompletionCallbacks and the pair of methods that manage it. whenSafeBrowsingCheckCompletes() is written to be safe to call unconditionally: if safeBrowsingCheckOngoing() is already false the callback runs synchronously on the spot, otherwise it is parked in the vector. fireSafeBrowsingCheckCompletionCallbacks() drains via std::exchange(m_safeBrowsingCheckCompletionCallbacks, { }), which swaps an empty vector in before iterating — so a callback that appends another callback during the drain does not get run by that same drain, and does not invalidate the iteration.

The fire site lives in the lookup completion block inside WebPageProxy::beginSafeBrowsingCheck(). It is placed immediately above the pre-existing timed-out-warning branch and shares that branch's !navigation->safeBrowsingCheckOngoing() guard, which matters because a single navigation can have several checks outstanding at once — the redirect chain contributes one per hop. Draining only when the last check retires means a deferred download decision sees the aggregate verdict, not the first hop's.

  beginSafeBrowsingCheck() completion block
    ├─ (per-check bookkeeping, remove from m_ongoingSafeBrowsingChecks)
    ├─ if (!safeBrowsingCheckOngoing())
    │     fireSafeBrowsingCheckCompletionCallbacks()   ← added: release parked downloads
    └─ if (!safeBrowsingCheckOngoing() && safeBrowsingWarning()
           && safeBrowsingCheckTimedOut())
          showBrowsingWarning(...)                     ← pre-existing: retroactive cover

Both decidePolicyForNavigationAction() and decidePolicyForResponseShared() grow the same early branch: when policyAction == PolicyAction::Download and a check is still ongoing, everything downstream is wrapped in a whenSafeBrowsingCheckCompletes() lambda and the function returns. The lambda captures what the deferred decision needs to outlive the current stack frame — completionHandlerWrapper, frame, frameInfo, the protectedThis/protectedPageClient refs, plus request on the response path. Inside, if a BrowsingWarning has landed in the interim, a subframe gets interruptedForPolicyChangeError reported through didFailProvisionalNavigationWithError followed by PolicyAction::Ignore, while a main frame gets the browsing-warning title pushed onto PageLoadState and the interstitial presented, with ContinueUnsafeLoad::Yes mapping back to PolicyAction::Download and everything else to Ignore. No warning means the original completionHandlerWrapper(PolicyAction::Download) fires and the download starts as before.

One mechanical detail on the response path: frameInfo is dropped from completionHandlerWrapper's capture list and moved into the deferral lambda instead, since only the deferred error-reporting path still needs it.

Five API tests land in SafeBrowsing.mm, all built on a DelayedLookupContext class swizzle that injects an artificial lookup delay. DownloadDeferredAndBlockedBySafeBrowsingPostTimeout is the load-bearing one: it uses a 500 ms delay with the comment that this deliberately exceeds "the ~250ms listener timeout", proving the deferral waits on the real verdict rather than merely on the timeout expiring. The rest cover a clean download still proceeding, a subframe-initiated download being blocked, and the navigation-action (as opposed to navigation-response) download path.

Background

Safe Browsing in WebKit. When a navigation starts, the UI process asks the platform reputation service whether the destination URL is known-bad — on Apple platforms through SSBLookupContext, soft-linked in WebPageProxyCocoa.mm. The lookup is asynchronous. Its result, if it is a match, is stored on the navigation object as a BrowsingWarning via setSafeBrowsingWarning().

Where the state lives. API::Navigation is the UI-process object representing one navigation from start to finish; it outlives individual IPC round-trips and is the natural place to hang per-navigation state. It tracks outstanding lookups in a ListHashSet<size_t> m_ongoingSafeBrowsingChecks, and the no-argument safeBrowsingCheckOngoing() returns true while any check for that navigation is still in flight.

The fail-open timeout. WebKit does not block a load indefinitely on the reputation service. If the lookup outruns a short listener timeout, setSafeBrowsingCheckTimedOut() is recorded and the load proceeds; when the verdict finally arrives, the safeBrowsingCheckTimedOut() branch in the completion block presents the interstitial over whatever has already loaded. The design choice is explicit in the commit message — "we proceed with loading whenever safe browsing responses take too long. This is fine for typical webpages as they red screen will show up, but slightly after the page load started."

BrowsingWarning and the interstitial. BrowsingWarning is the object describing a reputation match. PageClient::showBrowsingWarning() presents it and answers with either a URL (the learn-more link the user clicked) or a ContinueUnsafeLoad enum carrying the user's choice to back out or proceed anyway.

PolicyAction. This is the enum by which the UI process answers a navigation policy question: Use to load, Ignore to abandon, Download to hand the response off to DownloadProxy and write it to disk. Download terminates the navigation — the provisional load ends and the download proceeds on its own track.

The two decision points. decidePolicyForNavigationAction() runs before the request goes out; an embedding app's decidePolicyForNavigationAction: delegate, or a download attribute on a link, can select Download there. decidePolicyForResponseShared() runs once response headers arrive, where the decidePolicyForNavigationResponse: delegate makes the call — and a server can steer that decision with Content-Disposition: attachment or an unhandled MIME type. Both live in the UI process, the privileged side of the split relative to sandboxed WebContent.

Function<void()> queues. WTF's Function is a type-erased callable, so a Vector<Function<void()>> is a list of pending continuations. std::exchange(vec, { }) swaps in a fresh empty vector and returns the old one, which is the standard idiom for draining such a list exactly once without re-entrancy hazards.

Analysis

The root cause is an ordering error dressed up as a null check: while the lookup is in flight, navigation->safeBrowsingWarning() returns null — not because the URL is clean, but because nobody has asked yet, or rather, nobody has answered yet.

  Pre-fix, PolicyAction::Download with lookup in flight:

  t=0    navigation starts ──► beginSafeBrowsingCheck() ──┐  (async, SSBLookupContext)
  t=~30  response headers arrive                          │
         decidePolicyForResponseShared() → Download       │
           safeBrowsingWarning()  == nullptr ─────────────┼─► read as "clean"
           completionHandlerWrapper(Download)             │
  t=~35  DownloadProxy owns it; provisional load gone     │
  t=250  listener timeout: setSafeBrowsingCheckTimedOut() │
  t=~600 ◄─────────── verdict: MATCH ─────────────────────┘
         safeBrowsingCheckTimedOut() branch → showBrowsingWarning()
           …over what? file already on disk.

Both policy functions read that null and fell straight through to completionHandlerWrapper(PolicyAction::Download). The download therefore started on the strength of a verdict that had not been computed. The timeline above is the whole bug: everything to the left of t=250 executes before the fail-open even engages, and the retroactive interstitial at the bottom has nothing left to interpose itself on — by then DownloadProxy owns the transfer and the provisional load that a red screen would have replaced is gone.

The reason the fail-open is normally sound is that a page load stays revocable for as long as the verdict may take. Paint an interstitial over it and the user has still not read a word of the malicious page. That reasoning is silently scoped to revocable outcomes, and PolicyAction::Download is the one outcome in the enum that isn't. Nothing in the pre-fix code carried that distinction — the Download case sat in the same fall-through as everything else.

The subframe half compounds it. The pre-fix interstitial path in decidePolicyForNavigationAction() is gated on frame type:

if (RefPtr safeBrowsingWarning = navigation->safeBrowsingWarning()) {
    navigation->setSafeBrowsingWarning(nullptr);
    if (frame->isMainFrame() && safeBrowsingWarning->url().isValid()) {
        // …present the interstitial

A warning that arrives in time for a subframe-initiated download has no display path, because the interstitial is anchored to the top-level document. The capability — initiating a download — is reachable from a nested context; the consent UI that guards it is not. That is the shape Apple's impact line describes as a malicious iframe using another website's download settings: the download decision is made under the embedding page's context, and the user sees a file arrive from the site they believe they are on.

An attacker's trigger is unremarkable, which is part of what makes this worth fixing:

1. Get any content loaded such that a navigation to an attacker-controlled URL begins — top-level or inside an iframe on a page the user trusts.
2. Serve that URL with Content-Disposition: attachment or a MIME type the embedder hands to WKNavigationResponsePolicyDownload (or use a download-attribute link for the navigation-action path).
3. Respond fast enough that headers reach decidePolicyForResponseShared() while safeBrowsingCheckOngoing() is still true — or simply outlast the ~250 ms listener timeout, which the reputation service itself will do for you on a cold cache or a congested link.
4. The null safeBrowsingWarning() reads as clean; the file is written.

Step 3 is what bounds the window, and the DownloadDeferredAndBlockedBySafeBrowsingPostTimeout test with its 500 ms delay confirms the bound is the timeout rather than any attacker-inaccessible condition. Note that the attacker does not need to win a race in the usual sense — the reputation lookup involves a network round trip that a locally-served or CDN-fronted response will frequently beat outright.

The fix inverts the ordering for the Download case only. The policy continuation is parked in m_safeBrowsingCheckCompletionCallbacks and runs only after fireSafeBrowsingCheckCompletionCallbacks() observes !safeBrowsingCheckOngoing(), at which point safeBrowsingWarning() carries a real answer: null now means clean. Main-frame matches get the interstitial they always should have had, with ContinueUnsafeLoad::Yes still able to map back to PolicyAction::Download — the user's override is preserved, just informed. Subframe matches get the branch the pre-fix code lacked entirely: interruptedForPolicyChangeError reported through didFailProvisionalNavigationWithError and PolicyAction::Ignore, so the download is refused outright rather than shown a warning that could never render.

Worth being precise about the cost of this direction. The deferral queue has a single drain site and it sits on the success path; Navigation::~Navigation() is defaulted. A lookup that never completes, or a navigation torn down mid-check by a competing navigation, would destroy the vector with its entries unrun — a policy decision that neither starts nor fails, and a CompletionHandler destroyed without invocation, which WebKit asserts on in debug builds. That is a fail-closed hang traded for a fail-open bypass, which is the right direction, but it is not a no-op.

The pre-fix code read "verdict hasn't arrived" as "verdict is clean", and applied a fail-open timeout to PolicyAction::Download — the one policy outcome no later interstitial can undo.

Insight

The interesting property is not the race but the mismatch between a fail-open policy and the revocability of the action it gates. WebKit's Safe Browsing design is deliberately non-blocking and leans entirely on the fact that a page load can be covered by an interstitial after it starts — the safeBrowsingCheckTimedOut() branch is exactly that retroactive cover, and it is a reasonable piece of engineering. What it cannot do is notice when a consumer of the same verdict has an irreversible effect. Whenever an asynchronous security oracle is allowed to time out permissively, every consumer of its verdict has to be re-examined individually for whether its action can still be undone; the ones that cannot are the bugs, and they will not look like bugs locally, because at each call site the code is just doing what the design said it could.

Audit directions

• Fail-open oracle feeding an irreversible consumer. The invariant is that fail-open is only sound where the guarded action stays reversible for as long as the verdict may take. Narrow: enumerate every read of navigation->safeBrowsingWarning() and safeBrowsingCheckOngoing() in Source/WebKit/UIProcess/WebPageProxy.cpp and WebPageProxyCocoa.mm, and for each ask whether the branch it guards leaves a provisional load that a later showBrowsingWarning() could still cover — PolicyAction::LoadWillContinueInAnotherProcess and the QuickLook/PreviewConverter response path are the first two to check. Wider: the same shape appears in any WebKit UI-process gate that resolves asynchronously while the load continues — NetworkExtensionContentFilter verdicts, content-blocker evaluation, app-bound-domain and enhanced-security checks; the tell in code search is a completion block that both records a timeout flag and takes a late remediation branch. Widest: this is soft-fail security-oracle design generally — Chromium's SafeBrowsing check delegate timeout, TLS OCSP soft-fail, async malware scanning in mail and file-sync pipelines. The rule to carry across codebases: for each consumer of the oracle, name the action you would have to undo if the verdict arrives late; if you cannot name an undo, the oracle must block rather than time out.
• Security UI gated on frame type while the capability is not. The invariant is that a subframe must not reach an outcome whose only user-facing safeguard is main-frame-only. Narrow: grep Source/WebKit/UIProcess/WebPageProxy.cpp and the PageClient implementations for isMainFrame() conditions guarding interstitial display, warning titles, or PageLoadState transitions — the pre-fix if (frame->isMainFrame() && safeBrowsingWarning->url().isValid()) is the template, and this patch's if (!frame->isMainFrame()) early-Ignore is the corrected shape. Wider: any effect a subframe can initiate but only a main frame can be warned about — download initiation, external-URL and app-scheme launches (shouldOpenExternalURLsPolicy), permission and credential prompts; the tell is a security decision whose effect path has no frame-type condition while its notification path does. Widest: the general "nested context inherits the parent's privilege but not the parent's warning surface" class — browser extension frames, embedded WebViews in native apps, Chromium's own frame-scoped permission prompts, and any UI where the prompt anchors to the top-level document while the capability does not. Match tell: capability reachable from an embedded context, consent UI attached to the embedder.
• Continuation queue with a single success-path drain. The invariant is that every parked continuation has at least one guaranteed path to invocation, including error, cancellation, and destruction. Narrow: trace m_safeBrowsingCheckCompletionCallbacks in Source/WebKit/UIProcess/API/APINavigation.{h,cpp} — its only drain is the fireSafeBrowsingCheckCompletionCallbacks() call added to beginSafeBrowsingCheck()'s completion block, and Navigation::~Navigation() is defaulted, so establish what happens to a queued policy CompletionHandler when a navigation is superseded or the page closes mid-lookup. This needs a runtime test with a lookup stub that never returns rather than static reading; the DelayedLookupContext swizzle in SafeBrowsing.mm is already the right harness. Wider: other UI-process callback vectors parked on API objects awaiting an external answer — permission request managers, authentication challenge listeners, policy listener proxies; the tell is a Vector<Function<...>> or Vector<CompletionHandler<...>> member whose only std::exchange or clear site sits inside one success-path callback. Widest: the general "promise with an unreachable reject path" class — JS promises resolved only in the happy path, Rust oneshot senders dropped without send, any deferred-continuation design. Match tell: count the call sites that drain the queue; if there is exactly one and it lives on the success path, the failure and teardown paths strand the continuation.