Chrome · Chromium › Keepalive
CVE-2026-79010
Logic Error in Chromium (Keepalive)
Overview
Medium
Severity
4.3
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- `KeepAliveURLLoaderService`
- a browser-process service (
content/browser/loader/keep_alive_url_loader_service.cc) that proxiesfetch(..., {keepalive: true})andfetchLater()requests so they can finish after the document that started them has been unloaded. - `FactoryContext`
- per-receiver state attached to each bound
network::mojom::URLLoaderFactory/blink::mojom::FetchLaterLoaderFactoryreceiver, holding the factory,weak_document_ptr,policy_container_host,network_isolation_keyand the UKM source id of the initiating document. - `URLLoaderFactory::Clone()`
- a Mojo method that lets the holder of a factory remote bind an extra receiver to the same browser-side implementation, producing an additional independent handle.
- `OnDidCommitNavigation()`
- the hook that, once the document’s navigation commits, fills the context’s
weak_document_ptrand network isolation key; this is how the service learns which document the factory belongs to.
Root Cause Analysis
Before the patch, the Clone override for both receiver sets built a brand-new heap object with std::make_unique<FactoryContext>(loader_factory_receivers_.current_context()), using the now-removed copy constructor FactoryContext(const std::unique_ptr<FactoryContext>&), which took a one-time snapshot of factory, weak_document_ptr, policy_container_host and network_isolation_key. Later updates made to the original context, such as OnDidCommitNavigation() (reached through the weak pointer handed to the navigation) and UpdateFactory(), never reached these copies. So a cloned receiver could be cut off from the lifetime and identity of the document that owned it. The service also had no check at request-creation time. CreateKeepAliveURLLoader always passed the renderer’s client remote into the new KeepAliveURLLoader, so a request started through a stashed clone after the document was gone still sent the full response back to the renderer. That broke the rule that keepalive requests outliving their document are finished by the browser alone, with no response delivered to the renderer.
The fix makes FactoryContext base::RefCounted and has Clone add a reference to the same context. It also adds a did_commit_navigation flag and a WasInitiatorDocumentDestroyed() predicate, which CreateKeepAliveURLLoader uses to client.reset() before building the loader. The network request still runs, but the response is consumed in the browser.
Key insight
Factory clones copied the document-binding state by value instead of sharing it, and nothing checked whether the initiator document was still alive before wiring the renderer’s
URLLoaderClient to a new keepalive loader. The fix shares one ref-counted FactoryContext across all clones and drops the client when that shared context reports the document has been destroyed.Attack Path
- Obtain a keepalive factory
The renderer gets a keepalive
URLLoaderFactory(orFetchLaterLoaderFactory) receiver that the browser binds when the document commits. - Clone and stash the handle
The renderer calls
Clone()on that factory. Before the fix, the browser attached an independentFactoryContextcopy to the new receiver, and that copy did not follow later document updates. The regression tests clone either before or after the document is deleted. - Initiator document is torn down
The document that owns the factory navigates away or is destroyed (
DeleteContents()in the tests). The cloned Mojo pipe stays connected. - Issue a post-unload request
The renderer calls
CreateLoaderAndStarton the cloned factory with its ownURLLoaderClient. Before the fix,CreateKeepAliveURLLoaderforwardedOnReceiveResponseand the body to that client. Driving these Mojo calls directly on a stashed handle most likely needs a compromised renderer (inferred). - Receive response data without a live document The renderer gets response data for a request made under the dead document’s factory, policy container and isolation context, after the browser should have stopped serving that document (inferred consequence).
Impact Assessment
A renderer that keeps a cloned keepalive factory handle could keep making network requests with the factory and context of a document that has already been unloaded, and read the full responses in the renderer process. This bypasses the lifecycle limit that keepalive requests are only finished in the browser once the initiator is gone. The precondition is control over Mojo calls on a cloned factory handle after unload, which in practice suggests a compromised renderer (inferred); the advisory lists only a generic “Operation” impact and says nothing about severity or in-the-wild exploitation. Any concrete data-leak or policy-bypass outcome beyond “response data forwarded after unload” is
(inferred).Changed Functions
| Function | Change | Notes |
|---|---|---|
network_isolation_keycontent/browser/loader/keep_alive_url_loader_service.cc |
modified | |
BindNewEndpointAndPassReceivercontent/browser/loader/keep_alive_url_loader_service_unittest.cc |
modified | |
TEST_Fcontent/browser/loader/keep_alive_url_loader_service_unittest.cc |
modified |
Files Changed
content/browser/loader/keep_alive_url_loader_service_unittest.cccontent/browser/loader/keep_alive_url_loader_service.hcontent/browser/loader/keep_alive_url_loader_service.cc
Audit Directions
- Per-receiver context copied on `Clone()`Look for other
mojo::ReceiverSet<..., std::unique_ptr<Context>>users whoseClonecopies the context. Copied security state (document, origin, isolation key, lifetime flags) silently diverges from the original when the original is updated or invalidated. - Lifetime checks missing at request creationIn browser-side proxies that outlive a frame (keepalive,
fetchLater, beacons, service-worker or worker factories), check that every entry point re-validates the initiatorRenderFrameHost/document before returning data to the renderer, not only at bind time. - "Never set" vs "invalidated" weak pointersAny check based on
WeakDocumentPtrorAsRenderFrameHostIfValid()should tell an unset pointer apart from an invalidated one. Also review whatWasInitiatorDocumentDestroyed()returns for contexts whereOnDidCommitNavigation()never runs (it returns false).
Patch
From be963bc8c5c325b5adcd24b805c607a34b4656bc Mon Sep 17 00:00:00 2001 From: Ming-Ying Chung <[email protected]> Date: Tue, 04 Aug 2026 23:11:53 -0700 Subject: [PATCH] [keepalive] Don't forward responses after initiator is gone KeepAliveURLLoaderService keeps fetch keepalive requests alive in the browser process when the initiating document is unloaded so that network requests can complete. However, when a renderer created a clone of a factory handle via URLLoaderFactory::Clone(), the browser instantiated a separate FactoryContext copy for the cloned Mojo receiver. Because these cloned contexts were independent heap objects, the browser could not track or enforce document destruction status across cloned factory receivers. Consequently, a renderer holding a stashed factory clone could issue new requests post-unloading and still receive full response data via URLLoaderClient. This CL converts FactoryContext into a reference-counted object `scoped_refptr<FactoryContext>`. When BindFactory() is called for a new document, a distinct FactoryContext instance is allocated for that document. Subsequent calls to Clone() share the existing FactoryContext instance across all cloned Mojo receivers of that factory. If a keepalive request is initiated after the document has been destroyed, KeepAliveURLLoaderService detects the document teardown via the shared FactoryContext and drops the renderer-side URLLoaderClient. The network request still executes in the browser to maintain keepalive semantics for racing requests, but response data is handled entirely in the browser process without forwarding data back to the renderer. Bug: 497869284 Change-Id: I0d25f4945ec7a18bd59e3fb0e5ce0f599c07aff4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8173827 Commit-Queue: Ming-Ying Chung <[email protected]> Reviewed-by: Adam Rice <[email protected]> Cr-Commit-Position: refs/heads/main@{#1673918} --- diff --git a/content/browser/loader/keep_alive_url_loader_service.cc b/content/browser/loader/keep_alive_url_loader_service.cc index 7c9a556d..83768c3 100644 --- a/content/browser/loader/keep_alive_url_loader_service.cc +++ b/content/browser/loader/keep_alive_url_loader_service.cc @@ -54,19 +54,12 @@ CHECK(policy_container_host); } -KeepAliveURLLoaderService::FactoryContext::FactoryContext( - const std::unique_ptr<FactoryContext>& other) - : factory(other->factory), - weak_document_ptr(other->weak_document_ptr), - ukm_source_id(other->ukm_source_id), - policy_container_host(other->policy_container_host), - network_isolation_key(other->network_isolation_key) {} - KeepAliveURLLoaderService::FactoryContext::~FactoryContext() = default; void KeepAliveURLLoaderService::FactoryContext::OnDidCommitNavigation( NavigationHandle* navigation_handle) { CHECK(navigation_handle); + did_commit_navigation = true; weak_document_ptr = navigation_handle->GetRenderFrameHost()->GetWeakDocumentPtr(); network_isolation_key = @@ -82,6 +75,11 @@ CHECK(policy_container_host); } +bool KeepAliveURLLoaderService::FactoryContext::WasInitiatorDocumentDestroyed() + const { + return did_commit_navigation && !weak_document_ptr.AsRenderFrameHostIfValid(); +} + void KeepAliveURLLoaderService::FactoryContext:: OnBeforeKeepAliveURLLoaderCreated( const network::ResourceRequest& resource_request) { @@ -203,7 +201,7 @@ // loader is ensured to exist. raw_ptr<KeepAliveURLLoader> CreateKeepAliveURLLoader( PendingReceiverType<Interface> receiver, - const std::unique_ptr<FactoryContext>& context, + const scoped_refptr<FactoryContext>& context, int32_t request_id, uint32_t options, const network::ResourceRequest& resource_request, @@ -232,7 +230,13 @@ context->OnBeforeKeepAliveURLLoaderCreated(resource_request); // Passes in the pending remote of `client` from a renderer so that `loader` - // can forward response back to the renderer. + // can forward response back to the renderer. If the initiator document has + // already been destroyed, drop `client` so that the response is handled + // entirely in the browser, the same as for a request whose renderer + // disconnects after starting it. + if (context->WasInitiatorDocumentDestroyed()) { + client.reset(); + } CHECK(context->policy_container_host); auto loader = std::make_unique<KeepAliveURLLoader>( request_id, options, resource_request, std::move(client), @@ -400,7 +404,7 @@ // Adds a new factory receiver to the set, binding the pending `receiver` // from to `this` with a new context that has frame-specific data and keeps // reference to `subresource_proxying_factory_bundle`. - auto context = std::make_unique<FactoryContext>( + auto context = base::MakeRefCounted<FactoryContext>( std::move(subresource_proxying_factory_bundle), std::move(policy_container_host)); auto weak_context = context->weak_ptr_factory.GetWeakPtr(); @@ -452,10 +456,8 @@ override { DCHECK_CURRENTLY_ON(BrowserThread::UI); - loader_factory_receivers_.Add( - this, std::move(receiver), - std::make_unique<FactoryContext>( - loader_factory_receivers_.current_context())); + loader_factory_receivers_.Add(this, std::move(receiver), + loader_factory_receivers_.current_context()); } private: @@ -467,7 +469,7 @@ // be removed once it is disconnected from the corresponding remote (usually // in a renderer). mojo::ReceiverSet<network::mojom::URLLoaderFactory, - std::unique_ptr<FactoryContext>> + scoped_refptr<FactoryContext>> loader_factory_receivers_; }; @@ -501,7 +503,7 @@ // Adds a new factory receiver to the set, binding the pending `receiver` // from to `this` with a new context that has frame-specific data and keeps // reference to `shared_url_loader_factory`. - auto context = std::make_unique<FactoryContext>( + auto context = base::MakeRefCounted<FactoryContext>( std::move(shared_url_loader_factory), std::move(policy_container_host)); auto weak_context = context->weak_ptr_factory.GetWeakPtr(); loader_factory_receivers_.Add(this, std::move(receiver), @@ -545,10 +547,8 @@ receiver) override { DCHECK_CURRENTLY_ON(BrowserThread::UI); - loader_factory_receivers_.Add( - this, std::move(receiver), - std::make_unique<FactoryContext>( - loader_factory_receivers_.current_context())); + loader_factory_receivers_.Add(this, std::move(receiver), + loader_factory_receivers_.current_context()); } private: @@ -557,7 +557,7 @@ // be removed once it is disconnected from the corresponding remote in a // renderer. mojo::AssociatedReceiverSet<blink::mojom::FetchLaterLoaderFactory, - std::unique_ptr<FactoryContext>> + scoped_refptr<FactoryContext>> loader_factory_receivers_; }; diff --git a/content/browser/loader/keep_alive_url_loader_service.h b/content/browser/loader/keep_alive_url_loader_service.h index 0a0ddea..e6f7010 100644 --- a/content/browser/loader/keep_alive_url_loader_service.h +++ b/content/browser/loader/keep_alive_url_loader_service.h @@ -9,6 +9,7 @@ #include <optional> #include "base/containers/lru_cache.h" +#include "base/memory/ref_counted.h" #include "base/memory/scoped_refptr.h" #include "content/browser/loader/keep_alive_url_loader.h" #include "content/common/content_export.h" @@ -58,17 +59,16 @@ // // A FactoryContext is created whenever `BindFactory()` or // `BindFetchLaterLoaderFactory()` is called by - // RenderFrameHostImpl::CommitNavigation(). It can also be cloned by the same - // corresponding renderer, or when new window or new child frame is created. + // RenderFrameHostImpl::CommitNavigation(). It can also be "cloned" using an + // additional reference by the same corresponding renderer, or when new window + // or new child frame is created. // // See `mojo::ReceiverSetBase` for more details. - struct CONTENT_EXPORT FactoryContext { + struct CONTENT_EXPORT FactoryContext + : public base::RefCounted<FactoryContext> { FactoryContext( scoped_refptr<network::SharedURLLoaderFactory> factory, scoped_refptr<PolicyContainerHost> frame_policy_container_host); - // Called when a factory is cloned by URLLoaderFactory::Clone(). - explicit FactoryContext(const std::unique_ptr<FactoryContext>& other); - ~FactoryContext(); // Not Copyable. FactoryContext(const FactoryContext&) = delete; FactoryContext& operator=(const FactoryContext&) = delete; @@ -76,6 +76,10 @@ // Updates `weak_document_ptr` and other document-related fields. void OnDidCommitNavigation(NavigationHandle* navigation_handle); + // Returns true if `OnDidCommitNavigation()` was called for this context but + // the document it referenced has since been navigated away or destroyed. + bool WasInitiatorDocumentDestroyed() const; + // Called when a `KeepAliveURLLoader` is about to create. // This updates RenderFrameHostImpl via `weak_document_ptr` about the // creation of a keepalive request. @@ -132,8 +136,17 @@ // network isolation key of the committed RenderFrameHostImpl. net::NetworkIsolationKey network_isolation_key; + // Whether `OnDidCommitNavigation()` has been called for this context. Used + // to distinguish `weak_document_ptr` never having been set from it having + // been invalidated. + bool did_commit_navigation = false; + // This must be the last member. base::WeakPtrFactory<FactoryContext> weak_ptr_factory{this}; + + private: + friend class base::RefCounted<FactoryContext>; + ~FactoryContext(); }; // `storage_partition` creates and owns the instance of this service. It diff --git a/content/browser/loader/keep_alive_url_loader_service_unittest.cc b/content/browser/loader/keep_alive_url_loader_service_unittest.cc index c6528d5..c74d69a 100644 --- a/content/browser/loader/keep_alive_url_loader_service_unittest.cc +++ b/content/browser/loader/keep_alive_url_loader_service_unittest.cc @@ -144,6 +144,10 @@ return remote_url_loader.is_connected(); } void reset_remote_url_loader() { remote_url_loader.reset(); } + void Clone(mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver) { + remote_url_loader_factory->Clone(std::move(receiver)); + } + void FlushForTesting() { remote_url_loader_factory.FlushForTesting(); } private: mojo::Remote<network::mojom::URLLoaderFactory> remote_url_loader_factory; @@ -182,6 +186,18 @@ return remote_fetch_later_loader_.is_connected(); } void reset_remote_fetch_later_loader() { remote_fetch_later_loader_.reset(); } + mojo::PendingAssociatedReceiver<blink::mojom::FetchLaterLoaderFactory> + BindNewEndpointAndPassReceiver() { + return remote_fetch_later_loader_factory_.BindNewEndpointAndPassReceiver(); + } + void Clone( + mojo::PendingAssociatedReceiver<blink::mojom::FetchLaterLoaderFactory> + receiver) { + remote_fetch_later_loader_factory_->Clone(std::move(receiver)); + } + void FlushForTesting() { + remote_fetch_later_loader_factory_.FlushForTesting(); + } private: mojo::AssociatedRemote<blink::mojom::FetchLaterLoaderFactory> @@ -565,6 +581,77 @@ EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u); } +// Verifies that keepalive requests initiated via a cloned `URLLoaderFactory` +// after the initiating document has been unloaded will start the network +// load, but will not forward response data back to the renderer +// `URLLoaderClient`. +TEST_F(KeepAliveURLLoaderServiceTest, + LoadRequestFromClonedFactoryAfterPageIsUnloaded) { + FakeRemoteURLLoaderFactory renderer_loader_factory; + MockReceiverURLLoaderClient renderer_loader_client; + BindKeepAliveURLLoaderFactory(renderer_loader_factory); + + FakeRemoteURLLoaderFactory cloned_factory; + renderer_loader_factory.Clone(cloned_factory.BindNewPipeAndPassReceiver()); + renderer_loader_factory.FlushForTesting(); + + // Deletes the current RenderFrameHost and then loads a keepalive request + // from the cloned factory. + DeleteContents(); + cloned_factory.CreateLoaderAndStart( + CreateResourceRequest(GURL(kTestRequestUrl)), + renderer_loader_client.BindNewPipeAndPassRemote(), + /*expect_success=*/true); + + EXPECT_EQ(network_url_loader_factory().NumPending(), 1); + EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u); + + // When a response is received, it should not be forwarded to the client + // since the initiator document has been unloaded. + EXPECT_CALL(renderer_loader_client, OnReceiveResponse(_, _, _)).Times(0); + GetLastPendingRequest()->client->OnReceiveResponse( + CreateResponseHead({{kTestResponseHeaderName, kTestResponseHeaderValue}}), + /*body=*/{}, std::nullopt); + EXPECT_TRUE(base::test::RunUntil( + [&]() { return loader_service().NumLoadersForTesting() == 0u; })); + EXPECT_EQ(loader_service().NumLoadersForTesting(), 0u); +} + +// Verifies that cloning a `URLLoaderFactory` after the initiating document +// has been unloaded correctly binds the new factory receiver to the shared +// `FactoryContext`. +TEST_F(KeepAliveURLLoaderServiceTest, CloneFactoryAfterPageIsUnloaded) { + FakeRemoteURLLoaderFactory renderer_loader_factory; + MockReceiverURLLoaderClient renderer_loader_client; + BindKeepAliveURLLoaderFactory(renderer_loader_factory); + + // Deletes the current RenderFrameHost first before cloning the factory. + DeleteContents(); + + FakeRemoteURLLoaderFactory cloned_factory; + renderer_loader_factory.Clone(cloned_factory.BindNewPipeAndPassReceiver()); + renderer_loader_factory.FlushForTesting(); + + cloned_factory.CreateLoaderAndStart( + CreateResourceRequest(GURL(kTestRequestUrl)), + renderer_loader_client.BindNewPipeAndPassRemote(), + /*expect_success=*/true); + + EXPECT_EQ(network_url_loader_factory().NumPending(), 1); + EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u); + + // When a response is received, it should not be forwarded to the client + // since the initiator document has been unloaded. + EXPECT_CALL(renderer_loader_client, OnReceiveResponse(_, _, _)).Times(0); + GetLastPendingRequest()->client->OnReceiveResponse( + CreateResponseHead({{kTestResponseHeaderName, kTestResponseHeaderValue}}), + /*body=*/{}, std::nullopt); + EXPECT_TRUE(base::test::RunUntil( + [&]() { return loader_service().NumLoadersForTesting() == 0u; })); + EXPECT_EQ(loader_service().NumLoadersForTesting(), 0u); +} + + // This test initially provides an unbind factory to KeepAliveURLLoaderService. // After that, provides a bound factory via UpdateFactory. TEST_F(KeepAliveURLLoaderServiceTest, LoadRequestAfterUpdateFactory) { @@ -618,6 +705,52 @@ } } +// Verifies that a cloned `URLLoaderFactory` shares its `FactoryContext` with +// the original receiver, ensuring that updates via `UpdateFactory()` apply to +// requests initiated from cloned factory handles. +TEST_F(KeepAliveURLLoaderServiceTest, + LoadRequestWithClonedFactoryAfterUpdateFactory) { + FakeRemoteURLLoaderFactory renderer_loader_factory; + + auto unbound_factory = + std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(); + scoped_refptr<PolicyContainerHost> policy_container_host = + static_cast<RenderFrameHostImpl*>(main_rfh()) + ->policy_container_host() + ->Clone(); + auto context = loader_service().BindFactory( + renderer_loader_factory.BindNewPipeAndPassReceiver(), + network::SharedURLLoaderFactory::Create(std::move(unbound_factory)), + policy_container_host); + GetNavigationRequest()->SetKeepAliveURLLoaderFactoryContextForTesting( + context); + pending_navigation_->Commit(); + + FakeRemoteURLLoaderFactory cloned_factory; + renderer_loader_factory.Clone(cloned_factory.BindNewPipeAndPassReceiver()); + renderer_loader_factory.FlushForTesting(); + + renderer_loader_factory.reset_remote_url_loader(); + mojo::Remote<network::mojom::URLLoaderFactory> factory; + network_url_loader_factory().Clone(factory.BindNewPipeAndPassReceiver()); + auto pending_factory = std::make_unique<blink::PendingURLLoaderFactoryBundle>( + factory.Unbind(), blink::PendingURLLoaderFactoryBundle::SchemeMap(), + blink::PendingURLLoaderFactoryBundle::OriginMap(), + /*local_resource_loader_config=*/nullptr, + /*bypass_redirect_checks=*/false); + context->UpdateFactory( + network::SharedURLLoaderFactory::Create(std::move(pending_factory))); + + { + MockReceiverURLLoaderClient renderer_loader_client; + cloned_factory.CreateLoaderAndStart( + CreateResourceRequest(GURL(kTestRequestUrl)), + renderer_loader_client.BindNewPipeAndPassRemote(), + /*expect_success=*/true); + EXPECT_EQ(network_url_loader_factory().NumPending(), 1); + } +} + TEST_F(KeepAliveURLLoaderServiceTest, ForwardOnReceiveResponse) { FakeRemoteURLLoaderFactory renderer_loader_factory; MockReceiverURLLoaderClient renderer_loader_client; @@ -1378,6 +1511,58 @@ EXPECT_EQ(network_url_loader_factory().NumPending(), 1); } +// Verifies that FetchLater requests initiated from a cloned +// `FetchLaterLoaderFactory` after the initiating document has been unloaded +// correctly inherit the document lifecycle state from the shared +// `FactoryContext`. +TEST_F(FetchLaterKeepAliveURLLoaderServiceTest, + LoadFetchLaterRequestFromClonedFactoryAfterPageIsUnloaded) { + FakeRemoteFetchLaterLoaderFactory renderer_loader_factory; ... (file diff truncated)
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/content/browser/loader/keep_alive_url_loader_service_unittest.cc b/content/browser/loader/keep_alive_url_loader_service_unittest.cc
index c6528d5..c74d69a 100644
--- a/content/browser/loader/keep_alive_url_loader_service_unittest.cc
+++ b/content/browser/loader/keep_alive_url_loader_service_unittest.cc
@@ -144,6 +144,10 @@
return remote_url_loader.is_connected();
}
void reset_remote_url_loader() { remote_url_loader.reset(); }
+ void Clone(mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver) {
+ remote_url_loader_factory->Clone(std::move(receiver));
+ }
+ void FlushForTesting() { remote_url_loader_factory.FlushForTesting(); }
private:
mojo::Remote<network::mojom::URLLoaderFactory> remote_url_loader_factory;
@@ -182,6 +186,18 @@
return remote_fetch_later_loader_.is_connected();
}
void reset_remote_fetch_later_loader() { remote_fetch_later_loader_.reset(); }
+ mojo::PendingAssociatedReceiver<blink::mojom::FetchLaterLoaderFactory>
+ BindNewEndpointAndPassReceiver() {
+ return remote_fetch_later_loader_factory_.BindNewEndpointAndPassReceiver();
+ }
+ void Clone(
+ mojo::PendingAssociatedReceiver<blink::mojom::FetchLaterLoaderFactory>
+ receiver) {
+ remote_fetch_later_loader_factory_->Clone(std::move(receiver));
+ }
+ void FlushForTesting() {
+ remote_fetch_later_loader_factory_.FlushForTesting();
+ }
private:
mojo::AssociatedRemote<blink::mojom::FetchLaterLoaderFactory>
@@ -565,6 +581,77 @@
EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u);
}
+// Verifies that keepalive requests initiated via a cloned `URLLoaderFactory`
+// after the initiating document has been unloaded will start the network
+// load, but will not forward response data back to the renderer
+// `URLLoaderClient`.
+TEST_F(KeepAliveURLLoaderServiceTest,
+ LoadRequestFromClonedFactoryAfterPageIsUnloaded) {
+ FakeRemoteURLLoaderFactory renderer_loader_factory;
+ MockReceiverURLLoaderClient renderer_loader_client;
+ BindKeepAliveURLLoaderFactory(renderer_loader_factory);
+
+ FakeRemoteURLLoaderFactory cloned_factory;
+ renderer_loader_factory.Clone(cloned_factory.BindNewPipeAndPassReceiver());
+ renderer_loader_factory.FlushForTesting();
+
+ // Deletes the current RenderFrameHost and then loads a keepalive request
+ // from the cloned factory.
+ DeleteContents();
+ cloned_factory.CreateLoaderAndStart(
+ CreateResourceRequest(GURL(kTestRequestUrl)),
+ renderer_loader_client.BindNewPipeAndPassRemote(),
+ /*expect_success=*/true);
+
+ EXPECT_EQ(network_url_loader_factory().NumPending(), 1);
+ EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u);
+
+ // When a response is received, it should not be forwarded to the client
+ // since the initiator document has been unloaded.
+ EXPECT_CALL(renderer_loader_client, OnReceiveResponse(_, _, _)).Times(0);
+ GetLastPendingRequest()->client->OnReceiveResponse(
+ CreateResponseHead({{kTestResponseHeaderName, kTestResponseHeaderValue}}),
+ /*body=*/{}, std::nullopt);
+ EXPECT_TRUE(base::test::RunUntil(
+ [&]() { return loader_service().NumLoadersForTesting() == 0u; }));
+ EXPECT_EQ(loader_service().NumLoadersForTesting(), 0u);
+}
+
+// Verifies that cloning a `URLLoaderFactory` after the initiating document
+// has been unloaded correctly binds the new factory receiver to the shared
+// `FactoryContext`.
+TEST_F(KeepAliveURLLoaderServiceTest, CloneFactoryAfterPageIsUnloaded) {
+ FakeRemoteURLLoaderFactory renderer_loader_factory;
+ MockReceiverURLLoaderClient renderer_loader_client;
+ BindKeepAliveURLLoaderFactory(renderer_loader_factory);
+
+ // Deletes the current RenderFrameHost first before cloning the factory.
+ DeleteContents();
+
+ FakeRemoteURLLoaderFactory cloned_factory;
+ renderer_loader_factory.Clone(cloned_factory.BindNewPipeAndPassReceiver());
+ renderer_loader_factory.FlushForTesting();
+
+ cloned_factory.CreateLoaderAndStart(
+ CreateResourceRequest(GURL(kTestRequestUrl)),
+ renderer_loader_client.BindNewPipeAndPassRemote(),
+ /*expect_success=*/true);
+
+ EXPECT_EQ(network_url_loader_factory().NumPending(), 1);
+ EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u);
+
+ // When a response is received, it should not be forwarded to the client
+ // since the initiator document has been unloaded.
+ EXPECT_CALL(renderer_loader_client, OnReceiveResponse(_, _, _)).Times(0);
+ GetLastPendingRequest()->client->OnReceiveResponse(
+ CreateResponseHead({{kTestResponseHeaderName, kTestResponseHeaderValue}}),
+ /*body=*/{}, std::nullopt);
+ EXPECT_TRUE(base::test::RunUntil(
+ [&]() { return loader_service().NumLoadersForTesting() == 0u; }));
+ EXPECT_EQ(loader_service().NumLoadersForTesting(), 0u);
+}
+
+
// This test initially provides an unbind factory to KeepAliveURLLoaderService.
// After that, provides a bound factory via UpdateFactory.
TEST_F(KeepAliveURLLoaderServiceTest, LoadRequestAfterUpdateFactory) {
@@ -618,6 +705,52 @@
}
}
+// Verifies that a cloned `URLLoaderFactory` shares its `FactoryContext` with
+// the original receiver, ensuring that updates via `UpdateFactory()` apply to
+// requests initiated from cloned factory handles.
+TEST_F(KeepAliveURLLoaderServiceTest,
+ LoadRequestWithClonedFactoryAfterUpdateFactory) {
+ FakeRemoteURLLoaderFactory renderer_loader_factory;
+
+ auto unbound_factory =
+ std::make_unique<network::WrapperPendingSharedURLLoaderFactory>();
+ scoped_refptr<PolicyContainerHost> policy_container_host =
+ static_cast<RenderFrameHostImpl*>(main_rfh())
+ ->policy_container_host()
+ ->Clone();
+ auto context = loader_service().BindFactory(
+ renderer_loader_factory.BindNewPipeAndPassReceiver(),
+ network::SharedURLLoaderFactory::Create(std::move(unbound_factory)),
+ policy_container_host);
+ GetNavigationRequest()->SetKeepAliveURLLoaderFactoryContextForTesting(
+ context);
+ pending_navigation_->Commit();
+
+ FakeRemoteURLLoaderFactory cloned_factory;
+ renderer_loader_factory.Clone(cloned_factory.BindNewPipeAndPassReceiver());
+ renderer_loader_factory.FlushForTesting();
+
+ renderer_loader_factory.reset_remote_url_loader();
+ mojo::Remote<network::mojom::URLLoaderFactory> factory;
+ network_url_loader_factory().Clone(factory.BindNewPipeAndPassReceiver());
+ auto pending_factory = std::make_unique<blink::PendingURLLoaderFactoryBundle>(
+ factory.Unbind(), blink::PendingURLLoaderFactoryBundle::SchemeMap(),
+ blink::PendingURLLoaderFactoryBundle::OriginMap(),
+ /*local_resource_loader_config=*/nullptr,
+ /*bypass_redirect_checks=*/false);
+ context->UpdateFactory(
+ network::SharedURLLoaderFactory::Create(std::move(pending_factory)));
+
+ {
+ MockReceiverURLLoaderClient renderer_loader_client;
+ cloned_factory.CreateLoaderAndStart(
+ CreateResourceRequest(GURL(kTestRequestUrl)),
+ renderer_loader_client.BindNewPipeAndPassRemote(),
+ /*expect_success=*/true);
+ EXPECT_EQ(network_url_loader_factory().NumPending(), 1);
+ }
+}
+
TEST_F(KeepAliveURLLoaderServiceTest, ForwardOnReceiveResponse) {
FakeRemoteURLLoaderFactory renderer_loader_factory;
MockReceiverURLLoaderClient renderer_loader_client;
@@ -1378,6 +1511,58 @@
EXPECT_EQ(network_url_loader_factory().NumPending(), 1);
}
+// Verifies that FetchLater requests initiated from a cloned
+// `FetchLaterLoaderFactory` after the initiating document has been unloaded
+// correctly inherit the document lifecycle state from the shared
+// `FactoryContext`.
+TEST_F(FetchLaterKeepAliveURLLoaderServiceTest,
+ LoadFetchLaterRequestFromClonedFactoryAfterPageIsUnloaded) {
+ FakeRemoteFetchLaterLoaderFactory renderer_loader_factory;
+ BindFetchLaterLoaderFactory(renderer_loader_factory);
+
+ FakeRemoteFetchLaterLoaderFactory cloned_factory;
+ renderer_loader_factory.Clone(
+ cloned_factory.BindNewEndpointAndPassReceiver());
+ renderer_loader_factory.FlushForTesting();
+
+ // Deletes the current RenderFrameHost and then loads a request via the cloned
+ // factory.
+ DeleteContents();
+ cloned_factory.CreateLoader(
+ CreateFetchLaterResourceRequest(GURL(kTestRequestUrl)),
+ /*expect_success=*/true);
+
+ EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u);
+ EXPECT_EQ(network_url_loader_factory().NumPending(), 0);
+}
+
+// Verifies that cloning a `FetchLaterLoaderFactory` after the initiating
+// document has been unloaded correctly binds the new factory receiver to the
+// shared `FactoryContext`, allowing requests to be created with the unloaded
+// document state.
+TEST_F(FetchLaterKeepAliveURLLoaderServiceTest,
+ CloneFetchLaterFactoryAfterPageIsUnloaded) {
+ FakeRemoteFetchLaterLoaderFactory renderer_loader_factory;
+ BindFetchLaterLoaderFactory(renderer_loader_factory);
+
+ // Deletes the current RenderFrameHost first before cloning the factory.
+ DeleteContents();
+
+ FakeRemoteFetchLaterLoaderFactory cloned_factory;
+ renderer_loader_factory.Clone(
+ cloned_factory.BindNewEndpointAndPassReceiver());
+ renderer_loader_factory.FlushForTesting();
+
+ // Loads a request via the cloned factory.
+ cloned_factory.CreateLoader(
+ CreateFetchLaterResourceRequest(GURL(kTestRequestUrl)),
+ /*expect_success=*/true);
+
+ EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u);
+ EXPECT_EQ(network_url_loader_factory().NumPending(), 0);
+}
+
+
class KeepAliveURLLoaderServiceRetryTest
: public KeepAliveURLLoaderServiceTestBase {
protected:
Loading diff…
Original Bug Report
reported by [email protected]
Capability leak: KeepAliveURLLoaderService clones allow persistent access to origin credentials
Project Fortify, an experimental security project, has identified the following potential security issue.
Overview: A compromised renderer can persistently maintain access to a victim origin’s credentials by ‘stashing’ clones of the KeepAliveURLLoaderService factories. These clones are not invalidated when the originating document is destroyed, allowing an attacker to issue authenticated keep-alive requests long after the victim document is gone.
Affected files:
content/browser/loader/keep_alive_url_loader_service.cccontent/browser/loader/keep_alive_url_loader_service.h
Estimated timestamp from git blame: 2023-10-10
Summary
The KeepAliveURLLoaderService manages URLLoaderFactory and FetchLaterLoaderFactory instances used for keep-alive requests (e.g., fetch(..., {keepalive: true})). These factories are bound to a specific origin and its credentials via a network::SharedURLLoaderFactory. When a renderer calls Clone() on these factories, the browser creates a new Mojo receiver associated with a copy of the FactoryContext. However, these clones are independent and not collectively managed or invalidated. A compromised renderer can exploit this to maintain persistent, unauthorized access to a victim origin’s capabilities long after the victim document has been navigated away or destroyed.
Technical Details
In content/browser/loader/keep_alive_url_loader_service.cc, both KeepAliveURLLoaderFactories::Clone and FetchLaterLoaderFactories::Clone use the FactoryContext copy constructor to create a new context for the cloned receiver:
void Clone(mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver)
override {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
loader_factory_receivers_.Add(
this, std::move(receiver),
std::make_unique<FactoryContext>(
loader_factory_receivers_.current_context()));
}
The FactoryContext copy constructor (lines 59-66) copies all fields, including the factory (a strong scoped_refptr<network::SharedURLLoaderFactory>) and the network_isolation_key.
Each receiver in the browser-side mojo::ReceiverSet owns its own FactoryContext. Although the context contains a weak_document_ptr, the service does not verify that the document is still alive when CreateLoaderAndStart (or CreateLoader) is called. The SharedURLLoaderFactory (which holds the Network Service’s origin lock and credentials) remains alive in the browser process as long as the Mojo pipe remains open.
Potential Attack Scenario
- A renderer process is compromised.
- The attacker navigates an iframe to a sensitive victim origin (e.g.,
https://victim.example). In scenarios like Android where Site Isolation is relaxed, this document may load in the compromised renderer. - The browser calls
KeepAliveURLLoaderService::BindFactoryto set up aURLLoaderFactoryfor keep-alive requests bound tovictim.example’s origin lock and credentials. - The attacker calls
Clone()on this factory multiple times and “stashes” the resulting Mojo remotes in the renderer. - The victim document is navigated away or destroyed (e.g., the iframe is navigated to
about:blank). Ordinarily, the renderer’s access to the origin’s capabilities should be revoked at this point. - However, because the cloned Mojo pipes remain open in the compromised renderer, the corresponding
FactoryContextobjects and their origin-lockedSharedURLLoaderFactoryreferences remain alive in the browser. - The attacker uses these stashed clones to issue authenticated
keepaliverequests tovictim.example. - Because
KeepAliveURLLoaderuses aForwardingClientto send response data back to the renderer, the attacker can also receive and read the cross-origin responses to these requests, completing the Site Isolation bypass.
Suggested Fix
The KeepAliveURLLoaderService should track all cloned receivers belonging to the same document context. Because WeakDocumentPtr becomes null and cannot be compared when the document is destroyed, the FactoryContext should store a stable identifier like a blink::DocumentToken. The service should actively observe document destruction (e.g., via RenderFrameHostObserver) and close all associated receivers for that DocumentToken when the originating document is destroyed or navigated.
Evaluated with Chrome root at commit: 4859e669de60239572397b559c17dfab074511e9
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.
References
On This Page