Chrome · UI
CVE-2026-13973
Logic Error in UI
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifchrome/browser/ui/content_settings/content_setting_bubble_model.cc |
modified | |
TEST_Fchrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc |
modified | |
GenericSensorContentSettingBubbleModelTestchrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc |
modified |
Files Changed
chrome/browser/ui/content_settings/content_setting_bubble_model.ccchrome/browser/ui/content_settings/content_setting_bubble_model.hchrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cccomponents/subresource_filter/content/browser/profile_interaction_manager.cccomponents/subresource_filter/content/browser/subresource_filter_content_settings_manager.cc
Patch
From be6a8b5a343fc4960ed17076d532e1ba39bc26bd Mon Sep 17 00:00:00 2001 From: Antonio Sartori <[email protected]> Date: Tue, 19 May 2026 06:53:35 -0700 Subject: [PATCH] Use correct page when committing ContentSettingBubbleModel This change stores a weak reference to the Page when a ContentSettingSubresourceFilterBubbleModel is created and reuses that Page for the whole lifetime of the ContentSettingSubresourceFilterBubbleModel, ensuring consistency if the Page changes. Bug: 513832989 Fixed: 513832989 Change-Id: If534e878e5a1755df42f2a80373dacd2beb16945 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7851144 Reviewed-by: Josh Karlin <[email protected]> Commit-Queue: Antonio Sartori <[email protected]> Cr-Commit-Position: refs/heads/main@{#1632821} --- diff --git a/chrome/browser/ui/content_settings/content_setting_bubble_model.cc b/chrome/browser/ui/content_settings/content_setting_bubble_model.cc index 3a2d7c8..92c9b645 100644 --- a/chrome/browser/ui/content_settings/content_setting_bubble_model.cc +++ b/chrome/browser/ui/content_settings/content_setting_bubble_model.cc @@ -65,6 +65,8 @@ #include "components/permissions/permissions_client.h" #include "components/strings/grit/components_strings.h" #include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h" +#include "components/subresource_filter/content/browser/content_subresource_filter_web_contents_helper.h" +#include "components/subresource_filter/content/browser/subresource_filter_content_settings_manager.h" #include "components/subresource_filter/core/browser/subresource_filter_constants.h" #include "components/subresource_filter/core/browser/subresource_filter_features.h" #include "components/url_formatter/elide_url.h" @@ -1508,7 +1510,11 @@ ContentSettingSubresourceFilterBubbleModel:: ContentSettingSubresourceFilterBubbleModel(Delegate* delegate, WebContents* web_contents) - : ContentSettingBubbleModel(delegate, web_contents) { + : ContentSettingBubbleModel(delegate, web_contents), + page_(web_contents->GetPrimaryPage().GetWeakPtr()), + page_url_(web_contents->GetPrimaryPage() + .GetMainDocument() + .GetLastCommittedURL()) { SetTitle(); SetMessage(); SetManageText(); @@ -1550,9 +1556,15 @@ void ContentSettingSubresourceFilterBubbleModel::CommitChanges() { if (is_checked_) { - subresource_filter::ContentSubresourceFilterThrottleManager::FromPage( - web_contents()->GetPrimaryPage()) - ->OnReloadRequested(); + if (page_ && page_->IsPrimary()) { + subresource_filter::ContentSubresourceFilterThrottleManager::FromPage( + *page_) + ->OnReloadRequested(); + } else { + subresource_filter::SubresourceFilterContentSettingsManager( + HostContentSettingsMapFactory::GetForProfile(GetProfile())) + .AllowlistSite(page_url_); + } } } diff --git a/chrome/browser/ui/content_settings/content_setting_bubble_model.h b/chrome/browser/ui/content_settings/content_setting_bubble_model.h index eed8fce..2a5891b 100644 --- a/chrome/browser/ui/content_settings/content_setting_bubble_model.h +++ b/chrome/browser/ui/content_settings/content_setting_bubble_model.h @@ -15,6 +15,7 @@ #include "base/auto_reset.h" #include "base/gtest_prod_util.h" #include "base/memory/raw_ptr.h" +#include "base/memory/weak_ptr.h" #include "base/scoped_observation.h" #include "build/build_config.h" #include "chrome/app/vector_icons/vector_icons.h" @@ -495,6 +496,8 @@ void OnLearnMoreClicked() override; void CommitChanges() override; + base::WeakPtr<content::Page> page_; + GURL page_url_; bool is_checked_ = false; }; diff --git a/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc b/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc index ad0021b1..6751eb1 100644 --- a/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc +++ b/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc @@ -51,6 +51,7 @@ #include "components/permissions/permission_decision_auto_blocker.h" #include "components/permissions/permission_recovery_success_rate_tracker.h" #include "components/strings/grit/components_strings.h" +#include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h" #include "components/url_formatter/elide_url.h" #include "content/public/browser/web_contents.h" #include "content/public/common/content_features.h" @@ -1113,6 +1114,43 @@ l10n_util::GetStringUTF16(IDS_ALWAYS_ALLOW_ADS)); } +TEST_F(ContentSettingBubbleModelTest, SubresourceFilterNavigated) { + base::HistogramTester histogram_tester; + GURL page_url("https://www.example.com"); + HostContentSettingsMap* settings_map = + HostContentSettingsMapFactory::GetForProfile(profile()); + EXPECT_EQ(CONTENT_SETTING_BLOCK, + settings_map->GetContentSetting(page_url, GURL(), + ContentSettingsType::ADS)); + WebContentsTester::For(web_contents())->NavigateAndCommit(page_url); + + std::unique_ptr<ContentSettingBubbleModel> content_setting_bubble_model( + new ContentSettingSubresourceFilterBubbleModel(nullptr, web_contents())); + + content_setting_bubble_model->OnManageCheckboxChecked(true); + + // Simulate navigation. + GURL new_page_url("https://new.example.com"); + WebContentsTester::For(web_contents())->NavigateAndCommit(new_page_url); + + // This should not crash or apply settings to the new page. + content_setting_bubble_model->CommitChanges(); + + EXPECT_EQ(CONTENT_SETTING_ALLOW, + settings_map->GetContentSetting(page_url, GURL(), + ContentSettingsType::ADS)); + EXPECT_EQ(CONTENT_SETTING_BLOCK, + settings_map->GetContentSetting(new_page_url, GURL(), + ContentSettingsType::ADS)); + + histogram_tester.ExpectBucketCount( + "SubresourceFilter.Actions2", + subresource_filter::SubresourceFilterAction::kDetailsShown, 1); + histogram_tester.ExpectBucketCount( + "SubresourceFilter.Actions2", + subresource_filter::SubresourceFilterAction::kAllowlistedSite, 1); +} + class GenericSensorContentSettingBubbleModelTest : public ContentSettingBubbleModelTest { public: diff --git a/components/subresource_filter/content/browser/profile_interaction_manager.cc b/components/subresource_filter/content/browser/profile_interaction_manager.cc index f6d92ca..bbef518 100644 --- a/components/subresource_filter/content/browser/profile_interaction_manager.cc +++ b/components/subresource_filter/content/browser/profile_interaction_manager.cc @@ -46,8 +46,6 @@ CHECK(page_); CHECK(page_->IsPrimary()); - ContentSubresourceFilterThrottleManager::LogAction( - SubresourceFilterAction::kAllowlistedSite); profile_context_->settings_manager()->AllowlistSite( page_->GetMainDocument().GetLastCommittedURL()); diff --git a/components/subresource_filter/content/browser/subresource_filter_content_settings_manager.cc b/components/subresource_filter/content/browser/subresource_filter_content_settings_manager.cc index 738909b..436bc65 100644 --- a/components/subresource_filter/content/browser/subresource_filter_content_settings_manager.cc +++ b/components/subresource_filter/content/browser/subresource_filter_content_settings_manager.cc @@ -20,6 +20,7 @@ #include "components/content_settings/core/common/content_settings_metadata.h" #include "components/content_settings/core/common/content_settings_types.h" #include "components/content_settings/core/common/content_settings_utils.h" +#include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h" #include "url/gurl.h" namespace subresource_filter { @@ -67,6 +68,8 @@ } void SubresourceFilterContentSettingsManager::AllowlistSite(const GURL& url) { + ContentSubresourceFilterThrottleManager::LogAction( + SubresourceFilterAction::kAllowlistedSite); settings_map_->SetContentSettingDefaultScope( url, GURL(), ContentSettingsType::ADS, CONTENT_SETTING_ALLOW); }
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc b/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc
index ad0021b1..6751eb1 100644
--- a/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc
+++ b/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc
@@ -51,6 +51,7 @@
#include "components/permissions/permission_decision_auto_blocker.h"
#include "components/permissions/permission_recovery_success_rate_tracker.h"
#include "components/strings/grit/components_strings.h"
+#include "components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.h"
#include "components/url_formatter/elide_url.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_features.h"
@@ -1113,6 +1114,43 @@
l10n_util::GetStringUTF16(IDS_ALWAYS_ALLOW_ADS));
}
+TEST_F(ContentSettingBubbleModelTest, SubresourceFilterNavigated) {
+ base::HistogramTester histogram_tester;
+ GURL page_url("https://www.example.com");
+ HostContentSettingsMap* settings_map =
+ HostContentSettingsMapFactory::GetForProfile(profile());
+ EXPECT_EQ(CONTENT_SETTING_BLOCK,
+ settings_map->GetContentSetting(page_url, GURL(),
+ ContentSettingsType::ADS));
+ WebContentsTester::For(web_contents())->NavigateAndCommit(page_url);
+
+ std::unique_ptr<ContentSettingBubbleModel> content_setting_bubble_model(
+ new ContentSettingSubresourceFilterBubbleModel(nullptr, web_contents()));
+
+ content_setting_bubble_model->OnManageCheckboxChecked(true);
+
+ // Simulate navigation.
+ GURL new_page_url("https://new.example.com");
+ WebContentsTester::For(web_contents())->NavigateAndCommit(new_page_url);
+
+ // This should not crash or apply settings to the new page.
+ content_setting_bubble_model->CommitChanges();
+
+ EXPECT_EQ(CONTENT_SETTING_ALLOW,
+ settings_map->GetContentSetting(page_url, GURL(),
+ ContentSettingsType::ADS));
+ EXPECT_EQ(CONTENT_SETTING_BLOCK,
+ settings_map->GetContentSetting(new_page_url, GURL(),
+ ContentSettingsType::ADS));
+
+ histogram_tester.ExpectBucketCount(
+ "SubresourceFilter.Actions2",
+ subresource_filter::SubresourceFilterAction::kDetailsShown, 1);
+ histogram_tester.ExpectBucketCount(
+ "SubresourceFilter.Actions2",
+ subresource_filter::SubresourceFilterAction::kAllowlistedSite, 1);
+}
+
class GenericSensorContentSettingBubbleModelTest
: public ContentSettingBubbleModelTest {
public:
Loading diff…
Original Bug Report
reported by [email protected]
Potential origin confusion in SubresourceFilter bubble allows persistent cross-origin ads allowlisting
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. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: A logic bug in the SubresourceFilter bubble model allows an attacker-controlled site to redirect a user’s intent to allowlist ads onto a different origin. This occurs because the bubble model live-reads the current primary page from the WebContents during its asynchronous closing process, potentially after a navigation has committed. This leads to a persistent cross-origin bypass of the subresource filter for an arbitrary target site.
Affected files:
chrome/browser/ui/content_settings/content_setting_bubble_model.ccchrome/browser/ui/views/content_setting_bubble_contents.cccomponents/subresource_filter/content/browser/profile_interaction_manager.cccomponents/subresource_filter/content/browser/subresource_filter_content_settings_manager.cc
Estimated timestamp from git blame: 2018-07-13
Root Cause Analysis
The ContentSettingSubresourceFilterBubbleModel::CommitChanges() function, located in chrome/browser/ui/content_settings/content_setting_bubble_model.cc, resolves the target Page at commit time from the live WebContents, rather than using a snapshot of the page URL taken when the bubble was first opened.
void ContentSettingSubresourceFilterBubbleModel::CommitChanges() {
if (is_checked_) {
subresource_filter::ContentSubresourceFilterThrottleManager::FromPage(
web_contents()->GetPrimaryPage())
->OnReloadRequested();
}
}
The UI host, ContentSettingBubbleContents, calls CommitChanges() from its WindowClosing() method. When a navigation occurs while the bubble is open, ContentSettingBubbleContents::PrimaryPageChanged calls GetWidget()->Close(). In the Chromium Views framework, Widget::Close() is asynchronous and posts a task to the message loop to perform the actual destruction.
By the time the window actually closes and WindowClosing() invokes CommitChanges(), a pending navigation may have already committed. Consequently, web_contents()->GetPrimaryPage() returns the new page. When OnReloadRequested() is subsequently called on the new page’s throttle manager, it incorrectly applies a persistent ALLOW setting for ContentSettingsType::ADS to the destination origin’s URL.
This behavior contrasts with other bubble models (e.g., ContentSettingStorageAccessBubbleModel), which correctly snapshot the relevant URL at construction time to prevent origin confusion.
Potential Attack Scenario
- A user visits an attacker-controlled origin (Origin A) that is on the Safe Browsing abusive-ads list. The subresource filter activates, and the “Ads blocked” indicator appears in the omnibox.
- The user clicks the indicator to open the Ads bubble.
- The user checks the “Always allow ads on this site” checkbox, which sets the model’s internal
is_checked_state to true. - Before the user closes the bubble, JavaScript on Origin A triggers a cross-origin navigation to a target origin (Origin B) (e.g.,
location.href = 'https://target.example/'). - The navigation to Origin B commits. This triggers
PrimaryPageChangedin the browser UI, which initiates an asynchronous widget close. - The close task executes, calling
CommitChanges(). Because Origin B is now the primary page, its URL is retrieved from theWebContents. - Chrome writes a persistent
ALLOWsetting forContentSettingsType::ADSfor Origin B and reloads it, effectively allowlisting the site for ads bypass indefinitely.
Impact
An attacker can cause a user to unintentionally grant a persistent allowlist exception for the subresource filter to an arbitrary origin. While the subresource filter is a UX/anti-abuse feature, this represents a cross-origin authorization bypass caused by origin confusion in a privileged browser process.
Suggested Fix
The ContentSettingSubresourceFilterBubbleModel should snapshot the URL or Origin of the page at construction time and use that cached value in CommitChanges(), rather than querying the WebContents for the live primary page. This pattern is already used in other content setting bubbles to ensure user intent is applied to the correct origin.
Evaluated with Chrome root at commit: 1a8d40fc44df2088d5945c0bf53584038aa1614a
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
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