CVE-2026-79237
Overview
Background
- Initiator policies / `PolicyContainerPolicies`
- the security policies (CSP, IP address space, secure-context state, Local Network Access posture, etc.) of the document that started a navigation, which govern both enforcement of the navigation and, for local URLs, inheritance into the new document.
- `InitiatorNavigationState` / `NavigationStateKeepAlive`
- browser-side state holding an initiator’s policies for an in-flight navigation, kept alive so it survives even if the initiating
RenderFrameHostImplgoes away. - `StoragePartition` / `StoragePartitionConfig`
- the isolation boundary for storage and process state; policies from one partition must not leak into a navigation destined for a different partition.
- `NavigationPolicyContainerBuilder`
- the component that computes the final
PolicyContainerPoliciesfor a navigation, including deciding whether to inherit the initiator’s policies for local (e.g.about:blank,about:srcdoc) URLs.
Root Cause Analysis
Initiator policies serve two distinct purposes — enforcement checks that decide whether a navigation may proceed (e.g. Local Network Access), and inheritance of policies into a new local-URL document — but the pre-patch code conflated them, storing NavigationStateKeepAlives in the StoragePartition and passing initiator policies to the NavigationPolicyContainerBuilder at construction time before the destination’s StoragePartitionConfig was known. This meant policies could be inherited even when the initiator and the navigation resolved to different storage partitions (an isolation violation), and conversely the coupling created situations where initiator policies were not reliably passed for enforcement, opening an LNA-bypass opportunity.
The fix relocates keep-alives to BrowserContextImpl (via navigation_state_keep_alive_map_ and keep_alive_handles_receiver_set_) so a registered InitiatorNavigationState is always available to the NavigationRequest, stores a cloned PolicyContainerPolicies snapshot in InitiatorNavigationStateImpl, and postpones inheritance in NavigationPolicyContainerBuilder::ComputeInheritedPolicies/ComputeFinalPolicies/ComputePolicies by passing initiator_policies explicitly after request start, when the StoragePartitionConfig can be computed and compared. Enforcement callsites (e.g. DeriveClientSecurityStateForRendererInitiatedNavigation) now always receive the initiator policies regardless of partition, while inheritance is gated on partition match. This separates “must always enforce” from “may inherit,” restoring both isolation and enforcement invariants.
StoragePartition was known; the fix always passes initiator policies for enforcement but only inherits them after confirming the storage partitions match.Attack Path
- Initiate a navigation as a privileged initiator
A document with restrictive/privileged policies (e.g. a specific IP address space / secure-context posture used for Local Network Access enforcement) initiates a renderer-initiated navigation, creating an
InitiatorNavigationState/NavigationStateKeepAlive. - Reach a StoragePartition mismatch
The navigation is steered to a URL whose computed
StoragePartitionConfigdiffers from the initiator’s, a case the pre-patch flow did not distinguish when deciding policy inheritance. - Improper inheritance or dropped enforcement Pre-patch, the builder either inherits the initiator’s policies across the mismatched partition (isolation break) or, due to the coupling, fails to pass initiator policies to the enforcement callsites, creating a Local Network Access enforcement bypass.
- Authorization bypass The resulting document runs with policies it should not have inherited, or a navigation that should have been blocked by LNA/security-state enforcement proceeds (inferred consequence, matching the “incorrect authorization” classification).
Impact Assessment
StoragePartition boundary or to be omitted from enforcement, leading to incorrect authorization in the browser process — for example a Local Network Access bypass or leakage of an initiator’s policy posture into a document in a different partition. The advisory classifies this as an authorization error / incorrect authorization in Navigation; no exploited-in-the-wild status is stated, and any concrete cross-origin resource access beyond the policy mishandling is (inferred). Preconditions are the ability to drive a navigation whose destination partition differs from the initiator’s.Changed Functions
| Function | Change | Notes |
|---|---|---|
VideoDecodePerfHistorycontent/browser/browser_context_impl.h |
modified | |
BrowsingDataRemoverImplcontent/browser/browser_context_impl.h |
modified | |
DownloadManagercontent/browser/browser_context_impl.h |
modified | |
InMemoryFederatedPermissionContextcontent/browser/browser_context_impl.h |
modified | |
NavigationStateKeepAlivecontent/browser/browser_context_impl.h |
modified | |
PermissionControllercontent/browser/browser_context_impl.h |
modified | |
PrefetchServicecontent/browser/browser_context_impl.h |
modified | |
StoragePartitionImplMapcontent/browser/browser_context_impl.h |
modified | |
InitiatorPoliciescontent/browser/renderer_host/navigation_policy_container_builder.cc |
modified | |
ParentPoliciescontent/browser/renderer_host/navigation_policy_container_builder.cc |
modified | |
ComputeInheritedPoliciescontent/browser/renderer_host/navigation_policy_container_builder.cc |
modified | |
ifcontent/browser/renderer_host/navigation_policy_container_builder.cc |
modified |
Files Changed
content/browser/renderer_host/render_frame_host_impl.hcontent/browser/renderer_host/local_network_access_util.hcontent/browser/renderer_host/render_frame_host_impl_unittest.cccontent/browser/renderer_host/navigation_policy_container_builder.hcontent/browser/renderer_host/navigation_state_keep_alive.hcontent/browser/storage_partition_impl.hcontent/browser/renderer_host/navigation_policy_container_builder_unittest.cccontent/browser/renderer_host/navigation_request.hcontent/browser/browser_context_impl.hcontent/browser/browser_context_impl.cccontent/browser/renderer_host/navigation_state_keep_alive.cccontent/browser/renderer_host/navigation_policy_container_builder_browsertest.cccontent/browser/renderer_host/render_frame_proxy_host.cccontent/browser/renderer_host/policy_container_host.hcontent/browser/renderer_host/render_frame_host_impl.cccontent/browser/renderer_host/navigation_request.cccontent/browser/renderer_host/initiator_navigation_state_impl.cccontent/browser/renderer_host/local_network_access_util.cccontent/browser/renderer_host/initiator_navigation_state_impl.hcontent/browser/navigation_browsertest.cccontent/browser/storage_partition_impl.cccontent/browser/renderer_host/navigation_policy_container_builder.cc
Audit Directions
- Enforce-vs-inherit conflationLook for places that treat one policy object as both an enforcement input and an inheritance source; the two need independent handling, especially across isolation boundaries.
- State keyed to StoragePartition vs. BrowserContextAudit lifetime/ownership of navigation-related state to confirm it lives at the correct isolation scope and that partition mismatches are checked before cross-partition data flows.
- Timing of policy computationVerify that decisions depending on the destination
StoragePartitionConfig(or other post-request-start data) are not made at construction time when that context is not yet available.
Patch
From 68478a66451131d2b6865c068049cf50b36b7c66 Mon Sep 17 00:00:00 2001 From: Camille Lamy <[email protected]> Date: Wed, 15 Jul 2026 07:32:39 -0700 Subject: [PATCH] Prevent initiator policies inheritance on StoragePartition mismatch This CL ensures that initiator policies are always passed to the NavigationRequest but prevents their inheritance in case of StoragePartition mismatch. To do this, NavigationStateKeepAlives are now stored in BrowserContext instead of StoragePartition. This ensures that a properly registered InitiatorNavigationState is always passed to the NavigationRequest. Because we always pass it to the NavigationRequest, we now distinguish between initiator policies that need to be checked to see if the navigation can proceed and initiator policies that can be inherited by a navigation to a local URL. Inheritance of initiator policies in the NavigationPolicyContainerBuilder is postponed until after request start, which allows to properly compute the StoragePartitionConfig for the navigation and to check whether it matches that of the initiator. If not, policies are not inherited. Finally, all other callsites in NavigationRequest that check initiator policies have been moved to using the InitiatorNavigationState's policies regardless of StoragePartition, as those callsites are implementations of the initiator policies's enforcement. They must always be passed the initiator policies least we create an opportunity for bypassing those policies. Bug: 499068536, 510258191 Change-Id: I94483ae5a9e88e199a7208aa83e82e3eac38bcb3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7964570 Reviewed-by: Alex Moshchuk <[email protected]> Commit-Queue: Camille Lamy <[email protected]> Cr-Commit-Position: refs/heads/main@{#1662581} --- diff --git a/content/browser/browser_context_impl.cc b/content/browser/browser_context_impl.cc index c9f1944..2954f976 100644 --- a/content/browser/browser_context_impl.cc +++ b/content/browser/browser_context_impl.cc @@ -23,6 +23,7 @@ #include "content/browser/in_memory_federated_permission_context.h" #include "content/browser/permissions/permission_controller_impl.h" #include "content/browser/preloading/prefetch/prefetch_service.h" +#include "content/browser/renderer_host/navigation_state_keep_alive.h" #include "content/browser/renderer_host/navigation_transitions/navigation_entry_screenshot_cache.h" #include "content/browser/renderer_host/navigation_transitions/navigation_entry_screenshot_manager.h" #include "content/browser/renderer_host/render_frame_host_impl.h" @@ -409,4 +410,32 @@ return btm_service_.get(); } +void BrowserContextImpl::RegisterKeepAliveHandle( + mojo::PendingReceiver<blink::mojom::NavigationStateKeepAliveHandle> + receiver, + std::unique_ptr<NavigationStateKeepAlive> handle) { + auto frame_token = static_cast<InitiatorNavigationStateImpl*>( + handle->initiator_navigation_state().get()) + ->frame_token(); + navigation_state_keep_alive_map_[frame_token] = handle.get(); + keep_alive_handles_receiver_set_.Add(std::move(handle), std::move(receiver)); +} + +NavigationStateKeepAlive* BrowserContextImpl::GetNavigationStateKeepAlive( + blink::LocalFrameToken frame_token) { + return base::FindPtrOrNull(navigation_state_keep_alive_map_, frame_token); +} + +void BrowserContextImpl::RemoveKeepAliveHandleFromMap( + blink::LocalFrameToken frame_token, + NavigationStateKeepAlive* keep_alive) { + // The NavigationStateKeepAlive associated with `frame_token` may have + // changed. Make sure the specified one is removed from the map. + auto it = navigation_state_keep_alive_map_.find(frame_token); + if (it != navigation_state_keep_alive_map_.end() && + it->second == keep_alive) { + navigation_state_keep_alive_map_.erase(it); + } +} + } // namespace content diff --git a/content/browser/browser_context_impl.h b/content/browser/browser_context_impl.h index e222665..617c6fc 100644 --- a/content/browser/browser_context_impl.h +++ b/content/browser/browser_context_impl.h @@ -16,6 +16,9 @@ #include "content/browser/btm/btm_service_impl.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/shared_cors_origin_access_list.h" +#include "mojo/public/cpp/bindings/unique_receiver_set.h" +#include "third_party/abseil-cpp/absl/container/flat_hash_map.h" +#include "third_party/blink/public/mojom/frame/remote_frame.mojom.h" namespace media { class VideoDecodePerfHistory; @@ -37,6 +40,7 @@ class BrowsingDataRemoverImpl; class DownloadManager; class InMemoryFederatedPermissionContext; +class NavigationStateKeepAlive; class PermissionController; class PrefetchService; class StoragePartitionImplMap; @@ -120,6 +124,25 @@ // removed. void WaitForBtmCleanupForTesting(); + // Store `receiver` and its corresponding `handle`. These will be kept alive + // as long as the remote endpoint of `receiver` is still alive on the renderer + // side. The receiver will be automatically deleted when the endpoint is + // disconnected. + void RegisterKeepAliveHandle( + mojo::PendingReceiver<blink::mojom::NavigationStateKeepAliveHandle> + receiver, + std::unique_ptr<NavigationStateKeepAlive> handle); + + // Get the NavigationStateKeepAlive associated with `frame_token`. See + // `navigation_state_keep_alive_map_`. + NavigationStateKeepAlive* GetNavigationStateKeepAlive( + blink::LocalFrameToken frame_token); + + // Removes the NavigationStateKeepAlive associated with `frame_token`. This + // should be called when the keep alive is destructed. + void RemoveKeepAliveHandleFromMap(blink::LocalFrameToken frame_token, + NavigationStateKeepAlive* keep_alive); + private: // Creates the media service for storing/retrieving WebRTC encoding and // decoding performance stats. Exposed here rather than StoragePartition @@ -178,6 +201,33 @@ scoped_refptr<storage::ExternalMountPoints> external_mount_points_; #endif + // Maps frame tokens to NavigationStateKeepAlives. There is one + // NavigationStateKeepAlive per LocalFrameToken. It's possible to have + // multiple keep alives per LocalFrameToken (e.g., multiple in-flight + // navigations per RenderFrameHost), but this map will store the most recent + // NavigationStateKeepAlive. + // In the case of multiple navigations for a RenderFrameHost, + // it is assumed that they are handled in order, with the latest navigation's + // keep alive storing the state for that RenderFrameHost. + // Note: This member must be above `keep_alive_handles_receiver_set_`. During + // destruction, when NavigationStateKeepAlives get removed from the receiver + // set, they will then remove themselves from + // `navigation_state_keep_alive_map_`, so this map must still be alive when + // that happens. + using TokenNavigationStateKeepAliveMap = + absl::flat_hash_map<blink::LocalFrameToken, NavigationStateKeepAlive*>; + TokenNavigationStateKeepAliveMap navigation_state_keep_alive_map_; + + // Active keepalive handles for in-flight navigations. They are retained + // on `BrowserContextImpl` because, by design, they may need to outlive the + // `RenderFrameHostImpl` that initiated the navigation, but shouldn't be used + // in a different BrowserContext. + // Note that this set may contain in-flight navigations for different + // RenderFrameHosts, and furthermore, there may even be multiple in-flight + // navigations for a single RenderFrameHost. + mojo::UniqueReceiverSet<blink::mojom::NavigationStateKeepAliveHandle> + keep_alive_handles_receiver_set_; + // TODO: crbug.com/40169693 - BrowserContext and BrowserContextImpl both have // WeakPtrFactories. Remove one once the inheritance is sorted out. base::WeakPtrFactory<BrowserContextImpl> weak_factory_{this}; diff --git a/content/browser/renderer_host/initiator_navigation_state_impl.cc b/content/browser/renderer_host/initiator_navigation_state_impl.cc index 620cbef..4a12de0 100644 --- a/content/browser/renderer_host/initiator_navigation_state_impl.cc +++ b/content/browser/renderer_host/initiator_navigation_state_impl.cc @@ -6,7 +6,6 @@ #include <utility> -#include "content/browser/renderer_host/policy_container_host.h" #include "content/browser/site_instance_group.h" #include "content/browser/site_instance_impl.h" @@ -15,13 +14,12 @@ InitiatorNavigationStateImpl::InitiatorNavigationStateImpl( const blink::LocalFrameToken& token, ChildProcessId process_id, - scoped_refptr<PolicyContainerHost> policy_container_host, + const PolicyContainerHost* policy_container_host, scoped_refptr<SiteInstanceImpl> site_instance) : frame_token_(token), process_id_(process_id), - policy_container_host_(std::move(policy_container_host)), + policy_container_policies_(policy_container_host->policies().Clone()), site_instance_(std::move(site_instance)) { - CHECK(policy_container_host_); CHECK(site_instance_); } diff --git a/content/browser/renderer_host/initiator_navigation_state_impl.h b/content/browser/renderer_host/initiator_navigation_state_impl.h index 955c1b4..f507afa 100644 --- a/content/browser/renderer_host/initiator_navigation_state_impl.h +++ b/content/browser/renderer_host/initiator_navigation_state_impl.h @@ -6,6 +6,7 @@ #define CONTENT_BROWSER_RENDERER_HOST_INITIATOR_NAVIGATION_STATE_IMPL_H_ #include "base/memory/scoped_refptr.h" +#include "content/browser/renderer_host/policy_container_host.h" #include "content/browser/storage_partition_impl.h" #include "content/public/browser/initiator_navigation_state.h" #include "content/public/common/child_process_id.h" @@ -29,10 +30,10 @@ blink::LocalFrameToken frame_token() const { return frame_token_; } - // The PolicyContainer of the RenderFrameHost. This should never be null. - PolicyContainerHost* policy_container_host() const { - CHECK(policy_container_host_); - return policy_container_host_.get(); + // The PolicyContainerPolicies of the document at the moment the + // InitiatorNavigationState was created. + const PolicyContainerPolicies& policy_container_policies() const { + return policy_container_policies_; } // The SiteInstance of the RenderFrameHost. This should never be null. @@ -44,11 +45,12 @@ private: friend class RenderFrameHostImpl; - InitiatorNavigationStateImpl( - const blink::LocalFrameToken& token, - ChildProcessId process_id, - scoped_refptr<PolicyContainerHost> policy_container_host, - scoped_refptr<SiteInstanceImpl> site_instance); + // `policy_container_host` is the PolicyContainer of the RenderFrameHost that + // creates this InitiatorNavigationStateImpl. It should never be null. + InitiatorNavigationStateImpl(const blink::LocalFrameToken& token, + ChildProcessId process_id, + const PolicyContainerHost* policy_container_host, + scoped_refptr<SiteInstanceImpl> site_instance); ~InitiatorNavigationStateImpl() override; @@ -56,11 +58,11 @@ const blink::LocalFrameToken frame_token_; // The process ID of the document this state is associated with. - ChildProcessId process_id_; + const ChildProcessId process_id_; - // The PolicyContainerHost of the document at the moment the + // A copy of the PolicyContainerPolicies of the document at the moment the // InitiatorNavigationState was created. - scoped_refptr<PolicyContainerHost> policy_container_host_; + const PolicyContainerPolicies policy_container_policies_; // The SiteInstance of the document that created the navigation. scoped_refptr<SiteInstanceImpl> site_instance_; diff --git a/content/browser/renderer_host/local_network_access_util.cc b/content/browser/renderer_host/local_network_access_util.cc index dff946a..a2c75a66 100644 --- a/content/browser/renderer_host/local_network_access_util.cc +++ b/content/browser/renderer_host/local_network_access_util.cc @@ -237,6 +237,27 @@ policies.document_isolation_policy); } +network::mojom::ClientSecurityStatePtr +DeriveClientSecurityStateForRendererInitiatedNavigation( + const PolicyContainerPolicies& initiator_policies, + LocalNetworkAccessRequestContext local_network_request_context) { + CHECK(local_network_request_context == + LocalNetworkAccessRequestContext::kMainFrameNavigation || + local_network_request_context == + LocalNetworkAccessRequestContext::kSubframeNavigation); + // COEP and DocumentIsolationPolicy get default values below because COEP and + // DocumentIsolationPolicy are not checked by Fetch during navigations. COEP + // will be checked in NavigationRequest::EnforceCOEP. DocumentIsolationPolicy + // does not apply to navigations. + return network::mojom::ClientSecurityState::New( + network::CrossOriginEmbedderPolicy(), + initiator_policies.is_web_secure_context, + initiator_policies.ip_address_space, + DeriveLocalNetworkAccessRequestPolicy(initiator_policies, + local_network_request_context), + network::DocumentIsolationPolicy()); +} + // Special chrome schemes cannot directly be categorized in // public/private/loopback address spaces using information from the network or // the PolicyContainer. We have to classify them manually. In its default state diff --git a/content/browser/renderer_host/local_network_access_util.h b/content/browser/renderer_host/local_network_access_util.h index 64f61ce3..812b956 100644 --- a/content/browser/renderer_host/local_network_access_util.h +++ b/content/browser/renderer_host/local_network_access_util.h @@ -56,6 +56,21 @@ const PolicyContainerPolicies& policies, LocalNetworkAccessRequestContext local_network_request_context); +// Used to create a ClientSecurityState for renderer-initiated navigations. +// Browser initiated navigations do not have a ClientSecurityState as we do not +// need to check web security policies during them (since they originate from +// the browser). +// `initiator_policies` are the policies of the initiator of the navigation, +// which we will use for enforcement during the navigation Fetch. Note that +// the initiator policies should always be passed, even if they might not be +// inherited by the navigation. Passing only inheritable policies might create +// an LNA bypass, which is why initiator policies should be passed for all +// renderer-initiated navigations. +network::mojom::ClientSecurityStatePtr CONTENT_EXPORT +DeriveClientSecurityStateForRendererInitiatedNavigation( + const PolicyContainerPolicies& initiator_policies, + LocalNetworkAccessRequestContext local_network_request_context); + // Determines the IP address space that should be associated to execution // contexts instantiated from a resource loaded from this `url` and the given // response. diff --git a/content/browser/renderer_host/navigation_policy_container_builder.cc b/content/browser/renderer_host/navigation_policy_container_builder.cc index 4aa7102..8fe5028 100644 --- a/content/browser/renderer_host/navigation_policy_container_builder.cc +++ b/content/browser/renderer_host/navigation_policy_container_builder.cc @@ -50,20 +50,13 @@ NavigationPolicyContainerBuilder::NavigationPolicyContainerBuilder( RenderFrameHostImpl* parent, - std::unique_ptr<PolicyContainerPolicies> initiator_policies, const FrameNavigationEntry* history_entry) : parent_policies_(GetParentPolicies(parent)), - initiator_policies_(std::move(initiator_policies)), history_policies_(GetHistoryPolicies(history_entry)) {} NavigationPolicyContainerBuilder::~NavigationPolicyContainerBuilder() = default; const PolicyContainerPolicies* -NavigationPolicyContainerBuilder::InitiatorPolicies() const { - return initiator_policies_.get(); -} - -const PolicyContainerPolicies* NavigationPolicyContainerBuilder::ParentPolicies() const { return parent_policies_.get(); } @@ -263,7 +256,9 @@ } PolicyContainerPolicies -NavigationPolicyContainerBuilder::ComputeInheritedPolicies(const GURL& url) { +NavigationPolicyContainerBuilder::ComputeInheritedPolicies( + const GURL& url, + const PolicyContainerPolicies* initiator_policies) { DCHECK(url.SchemeIsLocal()) << url << " should not inherit policies"; if (url.IsAboutSrcdoc()) { @@ -272,8 +267,8 @@ return parent_policies_->Clone(); } - if (initiator_policies_) { - return initiator_policies_->Clone(); + if (initiator_policies) { + return initiator_policies->Clone(); } return PolicyContainerPolicies(); @@ -281,6 +276,7 @@ PolicyContainerPolicies NavigationPolicyContainerBuilder::ComputeFinalPolicies( NavigationHandle* navigation_handle, + const PolicyContainerPolicies* initiator_policies, bool is_inside_mhtml, network::mojom::WebSandboxFlags frame_sandbox_flags, bool is_credentialless) { @@ -300,7 +296,7 @@ // history navigation we will have CSP: something twice. policies = history_policies_->Clone(); } else { - policies = ComputeInheritedPolicies(url); + policies = ComputeInheritedPolicies(url, initiator_policies); IncorporateDeliveredPoliciesForLocalURL(policies); // TODO(crbug.com/40053796): Persist the policy container for URLs with @@ -333,14 +329,15 @@ void NavigationPolicyContainerBuilder::ComputePolicies( NavigationHandle* navigation_handle, + const PolicyContainerPolicies* initiator_policies, bool is_inside_mhtml, network::mojom::WebSandboxFlags frame_sandbox_flags, bool is_credentialless, bool is_secure_context_root) { DCHECK(!HasComputedPolicies()); ComputeIsWebSecureContext(is_secure_context_root); - SetFinalPolicies(ComputeFinalPolicies(navigation_handle, is_inside_mhtml, - frame_sandbox_flags, + SetFinalPolicies(ComputeFinalPolicies(navigation_handle, initiator_policies, + is_inside_mhtml, frame_sandbox_flags, is_credentialless)); } diff --git a/content/browser/renderer_host/navigation_policy_container_builder.h b/content/browser/renderer_host/navigation_policy_container_builder.h index ead77d79..7226d56 100644 --- a/content/browser/renderer_host/navigation_policy_container_builder.h +++ b/content/browser/renderer_host/navigation_policy_container_builder.h @@ -48,13 +48,11 @@ // All arguments may be nullptr and need only outlive this call. ... (file diff truncated)
Regression Test / PoC
diff --git a/content/browser/navigation_browsertest.cc b/content/browser/navigation_browsertest.cc
index 228fb3f9..84354fa 100644
--- a/content/browser/navigation_browsertest.cc
+++ b/content/browser/navigation_browsertest.cc
@@ -37,6 +37,7 @@
#include "cc/test/pixel_test_utils.h"
#include "components/ukm/test_ukm_recorder.h"
#include "components/viz/common/frame_sinks/copy_output_result.h"
+#include "content/browser/browser_context_impl.h"
#include "content/browser/browser_url_handler_impl.h"
#include "content/browser/renderer_host/navigation_request.h"
#include "content/browser/renderer_host/navigation_state_keep_alive.h"
@@ -100,6 +101,7 @@
#include "net/base/load_flags.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/controllable_http_response.h"
+#include "net/test/embedded_test_server/default_handlers.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/expectation_handler.h"
#include "net/test/embedded_test_server/http_response.h"
@@ -4564,8 +4566,8 @@
// Expect at this point that a NavigationStateKeepAlive has been created for
// the form submission.
NavigationStateKeepAlive* keep_alive =
- current_frame_host()->GetStoragePartition()->GetNavigationStateKeepAlive(
- current_frame_host()->GetFrameToken());
+ BrowserContextImpl::From(current_frame_host()->GetBrowserContext())
+ ->GetNavigationStateKeepAlive(current_frame_host()->GetFrameToken());
ASSERT_TRUE(keep_alive);
// Disable ref counts on the process, which resets all ref counts to 0. This
diff --git a/content/browser/renderer_host/navigation_policy_container_builder_browsertest.cc b/content/browser/renderer_host/navigation_policy_container_builder_browsertest.cc
index b33e80db..d88b790 100644
--- a/content/browser/renderer_host/navigation_policy_container_builder_browsertest.cc
+++ b/content/browser/renderer_host/navigation_policy_container_builder_browsertest.cc
@@ -109,9 +109,9 @@
// tests of HistoryPolicies() in the same place.
IN_PROC_BROWSER_TEST_F(NavigationPolicyContainerBuilderBrowserTest,
HistoryPoliciesWithoutEntry) {
- EXPECT_THAT(NavigationPolicyContainerBuilder(nullptr, nullptr, nullptr)
- .HistoryPolicies(),
- IsNull());
+ EXPECT_THAT(
+ NavigationPolicyContainerBuilder(nullptr, nullptr).HistoryPolicies(),
+ IsNull());
}
// Verifies that HistoryPolicies() returns non-null during history navigation.
@@ -126,7 +126,7 @@
network::mojom::IPAddressSpace::kLoopback);
NavigationPolicyContainerBuilder builder(
- nullptr, nullptr, GetLastCommittedFrameNavigationEntry());
+ nullptr, GetLastCommittedFrameNavigationEntry());
EXPECT_THAT(builder.HistoryPolicies(), Pointee(Eq(ByRef(root_policies))));
}
@@ -151,7 +151,7 @@
// Now that we have set up a navigation entry with non-default policies, we
// can run the test itself.
NavigationPolicyContainerBuilder builder(
- nullptr, nullptr, GetLastCommittedFrameNavigationEntry());
+ nullptr, GetLastCommittedFrameNavigationEntry());
EXPECT_THAT(builder.HistoryPolicies(), Pointee(Eq(ByRef(root_policies))));
}
@@ -169,7 +169,7 @@
network::mojom::IPAddressSpace::kLoopback);
FrameNavigationEntry* entry = GetLastCommittedFrameNavigationEntry();
- NavigationPolicyContainerBuilder builder(nullptr, nullptr, entry);
+ NavigationPolicyContainerBuilder builder(nullptr, entry);
// Verify the state is correct before navigating away.
EXPECT_THAT(builder.HistoryPolicies(), Pointee(Eq(ByRef(root_policies))));
@@ -179,7 +179,7 @@
// Now that the FrameNavigationEntry is non-current, verify that it still has
// the builder.
EXPECT_NE(entry, GetLastCommittedFrameNavigationEntry());
- NavigationPolicyContainerBuilder builder2(nullptr, nullptr, entry);
+ NavigationPolicyContainerBuilder builder2(nullptr, entry);
EXPECT_THAT(builder2.HistoryPolicies(), Pointee(Eq(ByRef(root_policies))));
}
@@ -187,12 +187,12 @@
// containing a copy of the builder's final policies.
IN_PROC_BROWSER_TEST_F(NavigationPolicyContainerBuilderBrowserTest,
CreatePolicyContainerForBlink) {
- NavigationPolicyContainerBuilder builder(nullptr, nullptr, nullptr);
+ NavigationPolicyContainerBuilder builder(nullptr, nullptr);
builder.SetIPAddressSpace(network::mojom::IPAddressSpace::kPublic);
MockNavigationHandle navigation_handle(GURL(), nullptr);
- builder.ComputePolicies(&navigation_handle, false,
- network::mojom::WebSandboxFlags::kNone,
+ builder.ComputePolicies(&navigation_handle, /*initiator_policies=*/nullptr,
+ false, network::mojom::WebSandboxFlags::kNone,
/*is_credentialless=*/false,
/*is_secure_context_root=*/false);
@@ -226,10 +226,7 @@
network::mojom::IPAddressSpace::kLoopback;
NavigationPolicyContainerBuilder builder(
- nullptr, initiator_policies.ClonePtr(),
- GetLastCommittedFrameNavigationEntry());
-
- EXPECT_NE(*builder.HistoryPolicies(), *builder.InitiatorPolicies());
+ nullptr, GetLastCommittedFrameNavigationEntry());
PolicyContainerPolicies history_policies = builder.HistoryPolicies()->Clone();
@@ -239,7 +236,7 @@
builder.AddContentSecurityPolicy(MakeTestCSP());
MockNavigationHandle navigation_handle(AboutBlankUrl(), nullptr);
- builder.ComputePolicies(&navigation_handle, false,
+ builder.ComputePolicies(&navigation_handle, &initiator_policies, false,
network::mojom::WebSandboxFlags::kNone,
/*is_credentialless=*/false,
/*is_secure_context_root=*/false);
@@ -274,7 +271,7 @@
RenderFrameHostImpl* parent = root->child_at(0)->current_frame_host();
NavigationPolicyContainerBuilder builder(
- parent, nullptr, GetLastCommittedFrameNavigationEntry());
+ parent, GetLastCommittedFrameNavigationEntry());
EXPECT_NE(*builder.HistoryPolicies(), *builder.ParentPolicies());
@@ -286,8 +283,8 @@
builder.AddContentSecurityPolicy(MakeTestCSP());
MockNavigationHandle navigation_handle(AboutSrcdocUrl(), nullptr);
- builder.ComputePolicies(&navigation_handle, false,
- network::mojom::WebSandboxFlags::kNone,
+ builder.ComputePolicies(&navigation_handle, /*initiator_policies=*/nullptr,
+ false, network::mojom::WebSandboxFlags::kNone,
/*is_credentialless=*/false,
/*is_secure_context_root=*/false);
@@ -306,7 +303,7 @@
EXPECT_TRUE(NavigateToURLFromRenderer(root_frame_host(), AboutBlankUrl()));
NavigationPolicyContainerBuilder builder(
- nullptr, nullptr, GetLastCommittedFrameNavigationEntry());
+ nullptr, GetLastCommittedFrameNavigationEntry());
builder.ComputePoliciesForError();
@@ -327,13 +324,13 @@
EXPECT_TRUE(NavigateToURLFromRenderer(root_frame_host(), AboutBlankUrl()));
NavigationPolicyContainerBuilder builder(
- nullptr, nullptr, GetLastCommittedFrameNavigationEntry());
+ nullptr, GetLastCommittedFrameNavigationEntry());
PolicyContainerPolicies history_policies = builder.HistoryPolicies()->Clone();
MockNavigationHandle navigation_handle(AboutBlankUrl(), nullptr);
- builder.ComputePolicies(&navigation_handle, false,
- network::mojom::WebSandboxFlags::kNone,
+ builder.ComputePolicies(&navigation_handle, /*initiator_policies=*/nullptr,
+ false, network::mojom::WebSandboxFlags::kNone,
/*is_credentialless=*/false,
/*is_secure_context_root=*/false);
EXPECT_THAT(builder.HistoryPolicies(), Pointee(Eq(ByRef(history_policies))));
@@ -425,13 +422,13 @@
EXPECT_TRUE(NavigateToURLFromRenderer(root, AboutBlankUrl()));
NavigationPolicyContainerBuilder builder(
- nullptr, nullptr, GetLastCommittedFrameNavigationEntry());
+ nullptr, GetLastCommittedFrameNavigationEntry());
PolicyContainerPolicies history_policies = builder.HistoryPolicies()->Clone();
MockNavigationHandle navigation_handle(GURL("http://foo.test"), nullptr);
- builder.ComputePolicies(&navigation_handle, false,
- network::mojom::WebSandboxFlags::kNone,
+ builder.ComputePolicies(&navigation_handle, /*initiator_policies=*/nullptr,
+ false, network::mojom::WebSandboxFlags::kNone,
/*is_credentialless=*/false,
/*is_secure_context_root=*/false);
@@ -441,8 +438,8 @@
EXPECT_THAT(builder.HistoryPolicies(), Pointee(Eq(ByRef(history_policies))));
navigation_handle.set_url(AboutBlankUrl());
- builder.ComputePolicies(&navigation_handle, false,
- network::mojom::WebSandboxFlags::kNone,
+ builder.ComputePolicies(&navigation_handle, /*initiator_policies=*/nullptr,
+ false, network::mojom::WebSandboxFlags::kNone,
/*is_credentialless=*/false,
/*is_secure_context_root=*/false);
@@ -454,18 +451,17 @@
IN_PROC_BROWSER_TEST_F(NavigationPolicyContainerBuilderBrowserTest,
FinalPoliciesAboutBlankWithInitiator) {
RenderFrameHostImpl* initiator = root_frame_host();
- const PolicyContainerPolicies& initiator_policies =
- initiator->policy_container_host()->policies();
+ const PolicyContainerPolicies* initiator_policies =
+ initiator->policy_container_host()->policies_ptr();
- NavigationPolicyContainerBuilder builder(
- nullptr, initiator_policies.ClonePtr(), nullptr);
+ NavigationPolicyContainerBuilder builder(nullptr, nullptr);
MockNavigationHandle navigation_handle(AboutBlankUrl(), nullptr);
- builder.ComputePolicies(&navigation_handle, false,
+ builder.ComputePolicies(&navigation_handle, initiator_policies, false,
network::mojom::WebSandboxFlags::kNone,
/*is_credentialless=*/false,
/*is_secure_context_root=*/false);
- EXPECT_EQ(builder.FinalPolicies(), initiator_policies);
+ EXPECT_EQ(builder.FinalPolicies(), *initiator_policies);
}
// Verifies that when the URL of the document to commit is `blob:.*`, the
@@ -473,21 +469,20 @@
IN_PROC_BROWSER_TEST_F(NavigationPolicyContainerBuilderBrowserTest,
FinalPoliciesBlobWithInitiator) {
RenderFrameHostImpl* initiator = root_frame_host();
- const PolicyContainerPolicies& initiator_policies =
- initiator->policy_container_host()->policies();
+ const PolicyContainerPolicies* initiator_policies =
+ initiator->policy_container_host()->policies_ptr();
- NavigationPolicyContainerBuilder builder(
- nullptr, initiator_policies.ClonePtr(), nullptr);
+ NavigationPolicyContainerBuilder builder(nullptr, nullptr);
MockNavigationHandle navigation_handle(
GURL("blob:https://example.com/016ece86-b7f9-4b07-88c2-a0e36b7f1dd6"),
nullptr);
- builder.ComputePolicies(&navigation_handle, false,
+ builder.ComputePolicies(&navigation_handle, initiator_policies, false,
network::mojom::WebSandboxFlags::kNone,
/*is_credentialless=*/false,
/*is_secure_context_root=*/false);
- EXPECT_EQ(builder.FinalPolicies(), initiator_policies);
+ EXPECT_EQ(builder.FinalPolicies(), *initiator_policies);
}
// Verifies that when the URL of the document to commit is `about:blank`, the
@@ -499,14 +494,13 @@
PolicyContainerPolicies initiator_policies =
initiator->policy_container_host()->policies().Clone();
- NavigationPolicyContainerBuilder builder(
- nullptr, initiator_policies.ClonePtr(), nullptr);
+ NavigationPolicyContainerBuilder builder(nullptr, nullptr);
// Add some CSP.
network::mojom::ContentSecurityPolicyPtr test_csp = MakeTestCSP();
builder.AddContentSecurityPolicy(test_csp.Clone());
MockNavigationHandle navigation_handle(AboutBlankUrl(), nullptr);
- builder.ComputePolicies(&navigation_handle, false,
+ builder.ComputePolicies(&navigation_handle, &initiator_policies, false,
network::mojom::WebSandboxFlags::kNone,
/*is_credentialless=*/false,
/*is_secure_context_root=*/false);
@@ -515,62 +509,4 @@
EXPECT_EQ(builder.FinalPolicies(), initiator_policies);
}
-// After ComputePolicies() or ComputePoliciesForError(), the initiator policies
-// are still accessible.
-IN_PROC_BROWSER_TEST_F(NavigationPolicyContainerBuilderBrowserTest,
- AccessInitiatorAfterComputingPolicies) {
- RenderFrameHostImpl* initiator = root_frame_host();
- const PolicyContainerPolicies& initiator_policies =
- initiator->policy_container_host()->policies();
-
- NavigationPolicyContainerBuilder builder(
- nullptr, initiator_policies.ClonePtr(), nullptr);
-
- EXPECT_THAT(builder.InitiatorPolicies(),
- Pointee(Eq(ByRef(initiator_policies))));
-
- MockNavigationHandle navigation_handle(GURL("https://foo.test"), nullptr);
- builder.ComputePolicies(&navigation_handle, false,
- network::mojom::WebSandboxFlags::kNone,
- /*is_credentialless=*/false,
- /*is_secure_context_root=*/false);
- EXPECT_THAT(builder.InitiatorPolicies(),
- Pointee(Eq(ByRef(initiator_policies))));
-
- builder.ComputePoliciesForError();
- EXPECT_THAT(builder.InitiatorPolicies(),
- Pointee(Eq(ByRef(initiator_policies))));
-}
-
-// Verifies that the initiator policies are preserved on
-// ResetForCrossDocumentRestart.
-IN_PROC_BROWSER_TEST_F(NavigationPolicyContainerBuilderBrowserTest,
- ResetForCrossDocumentRestartInitiatorPolicies) {
- RenderFrameHostImpl* initiator = root_frame_host();
- const PolicyContainerPolicies& initiator_policies =
... (truncated)
Original Bug Report
CSP/PNA bypass via incorrect StoragePartition keep-alive lookup in NavigationRequest
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the security team.
Overview: NavigationRequest incorrectly initializes its internal site_info_ with a default StoragePartitionConfig. When resolving keep-alive security policies for an initiator frame that has been destroyed, it queries the default storage partition instead of the initiator’s actual non-default partition. This causes navigations initiated from non-default partitions (like WebViews or Isolated Web Apps) to silently lose their security policies, potentially bypassing Private Network Access and CSP restrictions.
Affected files:
content/browser/renderer_host/navigation_request.cccontent/browser/renderer_host/render_frame_host_impl.cccontent/browser/renderer_host/navigation_policy_container_builder.cc
Estimated timestamp from git blame: 2021-07-23
Description
A logic flaw exists in the initialization of NavigationRequest that causes it to lose track of an initiator’s security policies if the initiator resides in a non-default StoragePartition and is destroyed while the navigation is starting.
In the NavigationRequest constructor (content/browser/renderer_host/navigation_request.cc), the site_info_ member is initialized using a single-argument constructor: site_info_(frame_tree_node_->navigator().controller().GetBrowserContext()). This specific SiteInfo constructor (content/browser/site_info.cc) hardcodes the partition configuration to the default partition via StoragePartitionConfig::CreateDefault(browser_context).
Later in the NavigationRequest constructor, NavigationPolicyContainerBuilder is instantiated to build the security policies for the navigation. It attempts to fetch the initiator’s policies by calling GetStoragePartitionWithCurrentSiteInfo(). Because site_info_ contains the default config, this returns the default StoragePartition, rather than the actual partition where the navigation originated.
If the initiating frame has already been destroyed (e.g., a popup that closes itself during an unload handler), its policies are preserved in a keep-alive map within its actual StoragePartition. Because the builder queries the default partition’s keep-alive map instead, the lookup fails. The navigation proceeds with nullptr for its initiator policies, which degrades security bounds.
Security Impact
The silent dropping of initiator policies leads to the following potential security bypasses:
- Private Network Access (PNA) Bypass: In
NavigationRequest::BuildClientSecurityStateForNavigationFetch(), ifpolicy_container_builder_->InitiatorPolicies()is null, it returns a null security state. The network service subsequently skips PNA preflight checks (Result::kAllowedMissingClientSecurityState), allowing CSRF attacks against local network devices from restricted contexts. - CSP
form-actionBypass: DuringNavigationRequest::CheckCSPDirectives, ifinitiator_policiesis null, theform-actionchecks are completely skipped, allowing an attacker to bypass the initiator’s CSP. - Policy Inheritance Loss: Navigations to local-scheme URLs (e.g.,
about:blank,data:) will fail to inherit the initiator’s sandbox flags and referrer policies.
Potential Attack Scenario
Note: These are suggested steps to trigger the vulnerability. Our tooling agent does not have the ability to execute code to verify a working proof-of-concept.
- An attacker hosts malicious content within a container that uses a non-default
StoragePartition(e.g., a Chrome App<webview>, GuestView, or a Controlled Frame). - The attacker’s page opens a popup window, which resides in the same non-default partition.
- The popup registers a
pagehideorunloadhandler. - The popup closes itself (e.g.,
window.close()). - During the frame destruction sequence, the handler fires and initiates a cross-document navigation on its
openerframe, pointing to a restricted local network address (e.g.,opener.location.href = 'http://192.168.1.1/admin'). - The browser registers a keep-alive handle for the popup’s policies in the non-default partition, and the popup’s
RenderFrameHostis destroyed. - The
NavigationRequestfor the opener’s navigation is created. It queries the default partition for the popup’s keep-alive policies, fails to find them, and proceeds without initiator policies. - The network request to
192.168.1.1is dispatched without a PNA preflight, successfully exploiting the local device.
Suggested Fix
Update the initialization of site_info_ in the NavigationRequest constructor so that it does not blindly default to StoragePartitionConfig::CreateDefault().
Instead of initializing site_info_ directly with BrowserContext*, it should be constructed by carefully determining the correct StoragePartitionConfig for the ongoing navigation (e.g., utilizing GetSiteInfoForCommonParamsURL() or querying the current FrameTreeNode’s site instance configuration) before looking up the keep-alive handles in NavigationPolicyContainerBuilder.
Evaluated with Chrome root at commit: ff3d2b74fa39431785bd60e51463b08fcc71ee33
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.