Chrome · Browser › Permissions
CVE-2026-95294
Spoofing in Browser (Permissions)
Medium
watch
CVSS 5.4
chrome
Spoofing
📄 Reporter bug report
🔧 Commit mapped
🧬 Security UI spoofable via occlusion late-binding timing or geometry
Overview
Medium
Severity
5.4
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- Quiet permission bubble
- A low-visibility UI (
ContentSettingQuietRequestBubbleModel) Chrome shows instead of a modal prompt when notification or geolocation requests are auto-suppressed viaQuietUiReason. - `PermissionRequestManager`
- The per-
WebContentsobject that queues pending permission requests and exposes the currently active one throughRequests(). - Request preemption
- When a new permission request arrives while a bubble is open, the manager can swap the active request in
Requests()[0], so a stale bubble may point at a different request than the one it was created for. - `QuietUiReason`
- An enum recording why quiet UI was chosen (e.g.
kEnabledInPrefs,kTriggeredByCrowdDeny,kTriggeredDueToAbusiveRequests), used to select the bubble’s title, message, and button text.
Root Cause Analysis
The quiet permission bubble’s action handlers (such as OnManageButtonClicked) resolved the user’s click by re-fetching manager->Requests()[0] at click time rather than remembering which request the bubble was originally created for. This violated the invariant that a user action applies to the request the UI was displaying: if the original request was preempted by a newly arriving permission request while the bubble remained open, Requests()[0] now referenced the preempting request, and the click was silently applied to the wrong request.
The fix caches request_type_ and quiet_ui_reason_ at bubble creation, adds IsBoundRequestStillActive() to confirm the active request still matches both cached values before acting, and observes the manager via prm_observation_ so the bubble’s widget is closed on removal, finalization, decision, or preemption. Because a mismatched or absent bound request now aborts the action and closes the bubble, a stale bubble can no longer resolve an action against an unrelated request.
Key insight
The core mistake was late-binding the user’s action to whatever request happened to be active at click time instead of the request the bubble was built to represent; the fix binds the action to a cached request identity (
request_type_ plus quiet_ui_reason_) and revalidates it before executing.Attack Path
- Trigger a quiet bubble A malicious or abusive site issues a notification or geolocation permission request that Chrome suppresses into the quiet bubble, caching nothing about which request it represents in the pre-patch code.
- Preempt while open
Before the user interacts, the page (or a subsequent request) causes
PermissionRequestManagerto swap the active request, soRequests()[0]no longer refers to the request shown in the still-open bubble. - Redirect the user's click
When the user clicks Done, Cancel, or Manage on the visually stale bubble, the handler re-fetches
Requests()[0]and applies the action to the newly preempting request instead of the one displayed. - UI misrepresentation The user’s decision is attributed to a permission request they never saw described, granting or dismissing a request under a misleading label (inferred that this can steer an unintended grant/deny outcome).
Impact Assessment
An attacker gains UI misrepresentation in the Browser process: a user acting on a quiet permission bubble can have their click applied to a different, preempting permission request than the one shown, undermining the integrity of the permission decision. Preconditions are that a quiet permission bubble is open and a preempting request replaces the active one before the user clicks. The advisory classifies this as a
LogicError causing UI misrepresentation and states no exploited-in-the-wild status; escalation to a concrete unintended permission grant is (inferred).Changed Functions
| Function | Change | Notes |
|---|---|---|
switchchrome/browser/ui/content_settings/content_setting_bubble_model.cc |
modified | |
ifchrome/browser/ui/content_settings/content_setting_bubble_model.cc |
modified | |
OnPermissionRequestManagerDestructedchrome/browser/ui/content_settings/content_setting_bubble_model.cc |
modified | |
ContentSettingQuietRequestBubbleModelchrome/browser/ui/content_settings/content_setting_bubble_model.h |
modified | |
ifchrome/browser/ui/views/content_setting_bubble_contents.cc |
modified |
Files Changed
chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.ccchrome/browser/ui/content_settings/BUILD.gnchrome/browser/ui/content_settings/content_setting_bubble_model.hchrome/browser/ui/views/content_setting_bubble_contents.cccontent/browser/permissions/permission_service_impl.cccontent/browser/permissions/permission_service_impl_unittest.ccchrome/browser/ui/views/content_setting_bubble_contents.hcontent/browser/permissions/embedded_permission_control_checker_unittest.ccchrome/browser/ui/content_settings/content_setting_bubble_model.ccchrome/browser/ui/content_settings/fake_owner.hchrome/browser/ui/content_settings/fake_owner.cc
Audit Directions
- Late-fetched active stateSearch other bubble and dialog models that re-query
PermissionRequestManager::Requests()or similar live singletons at action time instead of binding to the request captured at creation. - Preemptible queued UIReview UI that stays open across
PermissionRequestManagerrequest swaps or finalization without observing the manager, since these can desynchronize what is shown from what is acted upon. - Unvalidated action handlersAudit
On*ButtonClickedhandlers across content-settings and permission surfaces for missing revalidation of the originating request before applying grant/deny/dismiss actions.
Patch
From ab1bf87fa194826dc8a88d756d01ce184b896754 Mon Sep 17 00:00:00 2001 From: Thomas Nguyen <[email protected]> Date: Mon, 31 Aug 2026 13:05:54 -0700 Subject: [PATCH] [permissions] Bind quiet permission bubble actions to original request Previously, the quiet permission bubble resolved user actions by dynamically fetching the currently active request from the PermissionRequestManager. If the original request was preempted by a new permission request while the bubble was still open, user actions (such as clicking Done or Cancel) would be incorrectly applied to the newly preempting request instead of the original one. To fix this, the model now caches the original request type and the quiet UI reason upon creation. Button click handlers now verify that the bound request is still active before executing. If the request is no longer active, the action is aborted and the bubble is closed. Additionally, we will close the bubble's widget whenever the prompt is removed, finalized, or preempted. This ensures the UI cannot fall out of sync. Fixed: 513992281 Change-Id: I3f2c54de7a11b1949b8fc0d26201dddbc403ac19 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8277932 Commit-Queue: Thomas Nguyen <[email protected]> Reviewed-by: Judith Hemp <[email protected]> Auto-Submit: Thomas Nguyen <[email protected]> Reviewed-by: Thomas Lukaszewicz <[email protected]> Cr-Commit-Position: refs/heads/main@{#1689183} --- 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 15c0cf6..046bc71 100644 --- a/chrome/browser/ui/content_settings/content_setting_bubble_model.cc +++ b/chrome/browser/ui/content_settings/content_setting_bubble_model.cc @@ -1764,10 +1764,11 @@ } CHECK_GT(manager->Requests().size(), 0u); DCHECK_EQ(manager->Requests().size(), 1u); - const permissions::RequestType request_type = - manager->Requests()[0]->request_type(); + request_type_ = manager->Requests()[0]->request_type(); + quiet_ui_reason_ = quiet_ui_reason; + prm_observation_.Observe(manager); int bubble_title_string_id = 0; - switch (request_type) { + switch (*request_type_) { case permissions::RequestType::kNotifications: bubble_title_string_id = IDS_NOTIFICATIONS_QUIET_PERMISSION_BUBBLE_TITLE; break; @@ -1778,12 +1779,12 @@ NOTREACHED(); } set_title(l10n_util::GetStringUTF16(bubble_title_string_id)); - switch (*quiet_ui_reason) { + switch (*quiet_ui_reason_) { case QuietUiReason::kEnabledInPrefs: - DCHECK(request_type == permissions::RequestType::kNotifications || - request_type == permissions::RequestType::kGeolocation); + DCHECK(*request_type_ == permissions::RequestType::kNotifications || + *request_type_ == permissions::RequestType::kGeolocation); set_message(l10n_util::GetStringUTF16( - request_type == permissions::RequestType::kNotifications + *request_type_ == permissions::RequestType::kNotifications ? IDS_NOTIFICATIONS_QUIET_PERMISSION_BUBBLE_DESCRIPTION : IDS_GEOLOCATION_QUIET_PERMISSION_BUBBLE_DESCRIPTION)); set_done_button_text(l10n_util::GetStringUTF16( @@ -1793,7 +1794,7 @@ base::UserMetricsAction("Notifications.Quiet.AnimatedIconClicked")); break; case QuietUiReason::kTriggeredByCrowdDeny: - DCHECK_EQ(request_type, permissions::RequestType::kNotifications); + DCHECK_EQ(*request_type_, permissions::RequestType::kNotifications); set_message(l10n_util::GetStringUTF16( IDS_NOTIFICATIONS_QUIET_PERMISSION_BUBBLE_CROWD_DENY_DESCRIPTION)); set_done_button_text(l10n_util::GetStringUTF16( @@ -1804,7 +1805,7 @@ break; case QuietUiReason::kTriggeredDueToAbusiveRequests: case QuietUiReason::kTriggeredDueToAbusiveContent: - DCHECK_EQ(request_type, permissions::RequestType::kNotifications); + DCHECK_EQ(*request_type_, permissions::RequestType::kNotifications); set_message(l10n_util::GetStringUTF16( IDS_NOTIFICATIONS_QUIET_PERMISSION_BUBBLE_ABUSIVE_DESCRIPTION)); // TODO(crbug.com/40131070): It is rather confusing to have the `Cancel` @@ -1819,7 +1820,7 @@ base::UserMetricsAction("Notifications.Quiet.StaticIconClicked")); break; case QuietUiReason::kTriggeredDueToDisruptiveBehavior: - DCHECK_EQ(request_type, permissions::RequestType::kNotifications); + DCHECK_EQ(*request_type_, permissions::RequestType::kNotifications); set_message(l10n_util::GetStringUTF16( IDS_NOTIFICATIONS_QUIET_PERMISSION_BUBBLE_DISRUPTIVE_DESCRIPTION)); set_cancel_button_text(l10n_util::GetStringUTF16( @@ -1837,7 +1838,7 @@ case QuietUiReason::kTriggeredDueToLackOfGesture: int bubble_message_string_id = 0; int bubble_done_button_string_id = 0; - switch (request_type) { + switch (*request_type_) { case permissions::RequestType::kNotifications: bubble_message_string_id = IDS_NOTIFICATIONS_QUIET_PERMISSION_BUBBLE_PREDICTION_SERVICE_DESCRIPTION; @@ -1871,21 +1872,57 @@ return this; } -void ContentSettingQuietRequestBubbleModel::OnManageButtonClicked() { +bool ContentSettingQuietRequestBubbleModel::IsBoundRequestStillActive() const { + if (!quiet_ui_reason_ || !request_type_) { + return false; + } permissions::PermissionRequestManager* manager = permissions::PermissionRequestManager::FromWebContents(web_contents()); - CHECK_GT(manager->Requests().size(), 0u); - DCHECK_EQ(manager->Requests().size(), 1u); + return manager && !manager->Requests().empty() && + manager->Requests()[0]->request_type() == *request_type_ && + manager->ReasonForUsingQuietUi() == quiet_ui_reason_; +} + +void ContentSettingQuietRequestBubbleModel::CloseBubble() { + if (owner()) { + owner()->CloseBubble(); + } +} + +void ContentSettingQuietRequestBubbleModel::OnPromptRemoved() { + CloseBubble(); +} + +void ContentSettingQuietRequestBubbleModel::OnRequestsFinalized() { + CloseBubble(); +} + +void ContentSettingQuietRequestBubbleModel::OnRequestDecided( + permissions::PermissionAction action) { + CloseBubble(); +} + +void ContentSettingQuietRequestBubbleModel:: + OnPermissionRequestManagerDestructed() { + prm_observation_.Reset(); + CloseBubble(); +} + +void ContentSettingQuietRequestBubbleModel::OnManageButtonClicked() { + if (!IsBoundRequestStillActive()) { + CloseBubble(); + return; + } + permissions::PermissionRequestManager* manager = + permissions::PermissionRequestManager::FromWebContents(web_contents()); manager->set_manage_clicked(); if (is_UMA_for_test) { // `delegate()->ShowContentSettingsPage` opens a new tab. It is not needed // for UMA tests. return; } - const permissions::RequestType request_type = - manager->Requests()[0]->request_type(); if (delegate()) { - switch (request_type) { + switch (*request_type_) { case permissions::RequestType::kNotifications: delegate()->ShowContentSettingsPage(ContentSettingsType::NOTIFICATIONS); break; @@ -1899,6 +1936,10 @@ } void ContentSettingQuietRequestBubbleModel::OnLearnMoreClicked() { + if (!IsBoundRequestStillActive()) { + CloseBubble(); + return; + } permissions::PermissionRequestManager* manager = permissions::PermissionRequestManager::FromWebContents(web_contents()); manager->set_learn_more_clicked(); @@ -1912,34 +1953,28 @@ // We only show learn more button for Notification quiet ui dialog when it // is triggered due to abusive requests or contents. We don't have any learn // more button for the geolocation quiet ui dialogs. - DCHECK_EQ( - permissions::PermissionRequestManager::FromWebContents(web_contents()) - ->Requests()[0] - ->request_type(), - permissions::RequestType::kNotifications); + DCHECK_EQ(*request_type_, permissions::RequestType::kNotifications); delegate()->ShowLearnMorePage(ContentSettingsType::NOTIFICATIONS); } } void ContentSettingQuietRequestBubbleModel::OnDoneButtonClicked() { + if (!IsBoundRequestStillActive()) { + CloseBubble(); + return; + } permissions::PermissionRequestManager* manager = permissions::PermissionRequestManager::FromWebContents(web_contents()); - CHECK_GT(manager->Requests().size(), 0u); DCHECK_EQ(manager->Requests().size(), 1u); - auto quiet_ui_reason = manager->ReasonForUsingQuietUi(); - const permissions::PermissionRequest& request = *manager->Requests()[0]; - DCHECK(quiet_ui_reason); - DCHECK(request.request_type() == permissions::RequestType::kNotifications || - request.request_type() == permissions::RequestType::kGeolocation); // GEOLOCATION_WITH_OPTIONS is currently not supported on desktop. // // TODO(crbug.com/430494523): Plumb through the selected PromptOptions once it // is. - CHECK_NE(request.GetContentSettingsType(), + CHECK_NE(manager->Requests()[0]->GetContentSettingsType(), ContentSettingsType::GEOLOCATION_WITH_OPTIONS); - switch (*quiet_ui_reason) { + switch (*quiet_ui_reason_) { case QuietUiReason::kEnabledInPrefs: case QuietUiReason::kTriggeredDueToLackOfGesture: case QuietUiReason::kTriggeredByCrowdDeny: @@ -1958,14 +1993,13 @@ } void ContentSettingQuietRequestBubbleModel::OnCancelButtonClicked() { - permissions::PermissionRequestManager* manager = - permissions::PermissionRequestManager::FromWebContents(web_contents()); - - auto quiet_ui_reason = manager->ReasonForUsingQuietUi(); - if (!quiet_ui_reason) { + if (!IsBoundRequestStillActive()) { + CloseBubble(); return; } - switch (*quiet_ui_reason) { + permissions::PermissionRequestManager* manager = + permissions::PermissionRequestManager::FromWebContents(web_contents()); + switch (*quiet_ui_reason_) { case QuietUiReason::kEnabledInPrefs: case QuietUiReason::kTriggeredDueToLackOfGesture: case QuietUiReason::kTriggeredByCrowdDeny: @@ -1976,8 +2010,7 @@ case QuietUiReason::kTriggeredDueToAbusiveRequests: case QuietUiReason::kTriggeredDueToAbusiveContent: case QuietUiReason::kTriggeredDueToDisruptiveBehavior: - CHECK_EQ(manager->Requests()[0]->request_type(), - permissions::RequestType::kNotifications); + DCHECK_EQ(*request_type_, permissions::RequestType::kNotifications); manager->Accept(/*prompt_options=*/std::monostate()); base::RecordAction( base::UserMetricsAction("Notifications.Quiet.ShowForSiteClicked")); 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 fca5c5c..690931b 100644 --- a/chrome/browser/ui/content_settings/content_setting_bubble_model.h +++ b/chrome/browser/ui/content_settings/content_setting_bubble_model.h @@ -26,6 +26,9 @@ #include "components/content_settings/core/common/content_settings.h" #include "components/content_settings/core/common/content_settings_types.h" #include "components/custom_handlers/protocol_handler.h" +#include "components/permissions/permission_request_manager.h" +#include "components/permissions/prediction_service/permission_ui_selector.h" +#include "components/permissions/request_type.h" #include "net/base/schemeful_site.h" #include "services/device/public/cpp/geolocation/buildflags.h" #include "third_party/blink/public/common/mediastream/media_stream_request.h" @@ -110,6 +113,7 @@ virtual void OnListItemAdded(const ListItem& item) {} virtual void OnListItemRemovedAt(int index) {} virtual int GetSelectedRadioOption() = 0; + virtual void CloseBubble() {} protected: virtual ~Owner() = default; @@ -261,6 +265,7 @@ content::WebContents* web_contents() const; Profile* GetProfile() const; Delegate* delegate() const { return delegate_; } + Owner* owner() const { return owner_; } int selected_item() const { return owner_->GetSelectedRadioOption(); } content::Page& GetPage() const; @@ -452,7 +457,9 @@ // (which display the current permission state after the user makes the initial // decision), this is shown before the user makes the first ever permission // decisions. -class ContentSettingQuietRequestBubbleModel : public ContentSettingBubbleModel { +class ContentSettingQuietRequestBubbleModel + : public ContentSettingBubbleModel, + public permissions::PermissionRequestManager::Observer { public: ContentSettingQuietRequestBubbleModel(Delegate* delegate, content::Page& page); @@ -467,12 +474,37 @@ private: void SetManageText(); + // Returns true if the request this bubble was created for is still the + // currently active quiet request. + bool IsBoundRequestStillActive() const; + + // Requests the bubble owner to close the bubble widget. + void CloseBubble(); + // ContentSettingBubbleModel: void OnManageButtonClicked() override; void OnLearnMoreClicked() override; void OnDoneButtonClicked() override; void OnCancelButtonClicked() override; ContentSettingQuietRequestBubbleModel* AsQuietRequestBubbleModel() override; + + // permissions::PermissionRequestManager::Observer: + void OnPromptRemoved() override; + void OnRequestsFinalized() override; + void OnRequestDecided(permissions::PermissionAction action) override; + void OnPermissionRequestManagerDestructed() override; + + // The request type and quiet-UI reason this bubble was created for. The + // button labels are derived from these values, so the click handlers must + // act on them rather than re-reading the current request, which may have + // changed since the bubble was opened. + std::optional<permissions::RequestType> request_type_; + std::optional<permissions::PermissionUiSelector::QuietUiReason> + quiet_ui_reason_; + + base::ScopedObservation<permissions::PermissionRequestManager, + permissions::PermissionRequestManager::Observer> + prm_observation_{this}; }; // The model for the deceptive content bubble. diff --git a/chrome/browser/ui/content_settings/fake_owner.cc b/chrome/browser/ui/content_settings/fake_owner.cc index 06fcd6fe4..480d46c 100644 --- a/chrome/browser/ui/content_settings/fake_owner.cc +++ b/chrome/browser/ui/content_settings/fake_owner.cc @@ -7,3 +7,7 @@ int FakeOwner::GetSelectedRadioOption() { return selected_radio_option_; } + +void FakeOwner::CloseBubble() { + was_closed_ = true; +} diff --git a/chrome/browser/ui/content_settings/fake_owner.h b/chrome/browser/ui/content_settings/fake_owner.h index b513ee3..3e9b30f 100644 --- a/chrome/browser/ui/content_settings/fake_owner.h +++ b/chrome/browser/ui/content_settings/fake_owner.h @@ -29,9 +29,13 @@ model_->CommitChanges(); } + void CloseBubble() override; + bool was_closed() const { return was_closed_; } + private: const raw_ref<ContentSettingBubbleModel> model_; int selected_radio_option_; + bool was_closed_ = false; }; #endif // CHROME_BROWSER_UI_CONTENT_SETTINGS_FAKE_OWNER_H_ diff --git a/chrome/browser/ui/views/content_setting_bubble_contents.cc b/chrome/browser/ui/views/content_setting_bubble_contents.cc index 7001167..d78ca9e 100644 --- a/chrome/browser/ui/views/content_setting_bubble_contents.cc +++ b/chrome/browser/ui/views/content_setting_bubble_contents.cc @@ -418,6 +418,12 @@ NOTREACHED(); } +void ContentSettingBubbleContents::CloseBubble() { + if (GetWidget()) { + GetWidget()->Close(); + } +} + std::u16string ContentSettingBubbleContents::GetWindowTitle() const { if (!content_setting_bubble_model_) { return std::u16string(); diff --git a/chrome/browser/ui/views/content_setting_bubble_contents.h b/chrome/browser/ui/views/content_setting_bubble_contents.h index b482a5ca..d8202c42 100644 --- a/chrome/browser/ui/views/content_setting_bubble_contents.h +++ b/chrome/browser/ui/views/content_setting_bubble_contents.h @@ -57,6 +57,7 @@ const ContentSettingBubbleModel::ListItem& item) override; void OnListItemRemovedAt(int index) override; int GetSelectedRadioOption() override; + void CloseBubble() override; void managed_button_clicked_for_test() { content_setting_bubble_model_->is_UMA_for_test = true; diff --git a/content/browser/permissions/permission_service_impl.cc b/content/browser/permissions/permission_service_impl.cc index 48de8e31..97e77edb 100644 --- a/content/browser/permissions/permission_service_impl.cc +++ b/content/browser/permissions/permission_service_impl.cc @@ -145,17 +145,33 @@ return false; } -// Helper check if permission types are all supported by Page Embedded -// Permission. +// Helper which returns true if all permission types match the embedded +// permission element type identified by `descriptor`. bool CheckPageEmbeddedPermissionTypes( ... (file diff truncated)
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.cc b/chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.cc
index 722fedf..9ea9608 100644
--- a/chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.cc
+++ b/chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.cc
@@ -31,7 +31,12 @@
#include "components/content_settings/core/test/content_settings_mock_provider.h"
#include "components/content_settings/core/test/content_settings_test_utils.h"
#include "components/network_session_configurator/common/network_switches.h"
+#include "components/permissions/permission_request_manager.h"
#include "components/permissions/permission_util.h"
+#include "components/permissions/prediction_service/permission_ui_selector.h"
+#include "components/permissions/request_type.h"
+#include "components/permissions/test/mock_permission_request.h"
+#include "components/permissions/test/mock_permission_ui_selector.h"
#include "components/strings/grit/components_strings.h"
#include "components/url_formatter/elide_url.h"
#include "content/public/common/content_switches.h"
@@ -377,3 +382,170 @@
histograms.ExpectTotalCount("ContentSettings.Popups", 5);
}
+
+class ContentSettingBubbleModelQuietRequestTest : public InProcessBrowserTest {
+ protected:
+ using QuietUiReason = permissions::PermissionUiSelector::QuietUiReason;
+ using Decision = permissions::PermissionUiSelector::Decision;
+ using MockRequestState =
+ permissions::MockPermissionRequest::MockPermissionRequestState;
+
+ void SetUpInProcessBrowserTestFixture() override {
+ https_server_ = std::make_unique<net::EmbeddedTestServer>(
+ net::EmbeddedTestServer::TYPE_HTTPS);
+ https_server_->ServeFilesFromSourceDirectory(GetChromeTestDataDir());
+ ASSERT_TRUE(https_server_->Start());
+ }
+
+ content::WebContents* OpenTab() {
+ GURL url(
+ https_server_->GetURL("/content_setting_bubble/mixed_script.html"));
+ EXPECT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+ return browser()->tab_strip_model()->GetActiveWebContents();
+ }
+
+ permissions::PermissionRequestManager* GetManager(
+ content::WebContents* web_contents) {
+ return permissions::PermissionRequestManager::FromWebContents(web_contents);
+ }
+
+ void AddQuietRequest(content::WebContents* web_contents,
+ permissions::RequestType request_type,
+ QuietUiReason reason,
+ MockRequestState* state) {
+ auto* manager = GetManager(web_contents);
+ manager->set_permission_ui_selector_for_testing(
+ std::make_unique<MockPermissionUiSelector>(
+ Decision::UseQuietUi(reason, std::nullopt)));
+ manager->AddRequest(web_contents->GetPrimaryMainFrame(),
+ std::make_unique<permissions::MockPermissionRequest>(
+ web_contents->GetLastCommittedURL(), request_type,
+ permissions::PermissionRequestGestureType::GESTURE,
+ state ? state->GetWeakPtr() : nullptr));
+ base::RunLoop().RunUntilIdle();
+ }
+
+ std::unique_ptr<ContentSettingBubbleModel> CreateBubble(
+ content::WebContents* web_contents) {
+ return std::make_unique<ContentSettingQuietRequestBubbleModel>(
+ BrowserContentSettingBubbleModelDelegate::From(browser()),
+ web_contents->GetPrimaryPage());
+ }
+
+ std::unique_ptr<net::EmbeddedTestServer> https_server_;
+};
+
+// The bubble model is created for a specific request and its button labels are
+// computed from that request's quiet-UI reason. If the request is preempted by
+// another one while the bubble is still open, the buttons must not act on the
+// new request.
+IN_PROC_BROWSER_TEST_F(ContentSettingBubbleModelQuietRequestTest,
+ DoneButtonIgnoresPreemptedRequest) {
+ content::WebContents* web_contents = OpenTab();
+
+ MockRequestState r1_state;
+ AddQuietRequest(web_contents, permissions::RequestType::kNotifications,
+ QuietUiReason::kTriggeredDueToAbusiveRequests, &r1_state);
+ ASSERT_EQ(GetManager(web_contents)->ReasonForUsingQuietUi(),
+ QuietUiReason::kTriggeredDueToAbusiveRequests);
+
+ auto bubble = CreateBubble(web_contents);
+ EXPECT_EQ(
+ bubble->bubble_content().done_button_text,
+ l10n_util::GetStringUTF16(
+ IDS_NOTIFICATIONS_QUIET_PERMISSION_BUBBLE_CONTINUE_BLOCKING_BUTTON));
+
+ MockRequestState r2_state;
+ AddQuietRequest(web_contents, permissions::RequestType::kGeolocation,
+ QuietUiReason::kEnabledInPrefs, &r2_state);
+ ASSERT_EQ(GetManager(web_contents)->Requests()[0]->request_type(),
+ permissions::RequestType::kGeolocation);
+
+ bubble->OnDoneButtonClicked();
+ EXPECT_FALSE(r2_state.granted);
+ EXPECT_FALSE(r2_state.finished);
+}
+
+IN_PROC_BROWSER_TEST_F(ContentSettingBubbleModelQuietRequestTest,
+ CancelButtonIgnoresPreemptedRequest) {
+ content::WebContents* web_contents = OpenTab();
+
+ AddQuietRequest(web_contents, permissions::RequestType::kGeolocation,
+ QuietUiReason::kEnabledInPrefs, nullptr);
+ auto bubble = CreateBubble(web_contents);
+
+ MockRequestState r2_state;
+ AddQuietRequest(web_contents, permissions::RequestType::kNotifications,
+ QuietUiReason::kTriggeredDueToAbusiveRequests, &r2_state);
+ ASSERT_EQ(GetManager(web_contents)->Requests()[0]->request_type(),
+ permissions::RequestType::kNotifications);
+
+ bubble->OnCancelButtonClicked();
+ EXPECT_FALSE(r2_state.granted);
+ EXPECT_FALSE(r2_state.finished);
+}
+
+IN_PROC_BROWSER_TEST_F(ContentSettingBubbleModelQuietRequestTest,
+ DoneButtonActsOnOriginalRequest) {
+ content::WebContents* web_contents = OpenTab();
+
+ MockRequestState r1_state;
+ AddQuietRequest(web_contents, permissions::RequestType::kNotifications,
+ QuietUiReason::kTriggeredDueToAbusiveRequests, &r1_state);
+
+ auto bubble = CreateBubble(web_contents);
+ bubble->OnDoneButtonClicked();
+ EXPECT_FALSE(r1_state.granted);
+ EXPECT_TRUE(r1_state.finished);
+}
+
+IN_PROC_BROWSER_TEST_F(ContentSettingBubbleModelQuietRequestTest,
+ CancelButtonActsOnOriginalRequest) {
+ content::WebContents* web_contents = OpenTab();
+
+ MockRequestState r1_state;
+ AddQuietRequest(web_contents, permissions::RequestType::kNotifications,
+ QuietUiReason::kTriggeredDueToAbusiveRequests, &r1_state);
+
+ auto bubble = CreateBubble(web_contents);
+ bubble->OnCancelButtonClicked();
+ EXPECT_TRUE(r1_state.granted);
+ EXPECT_TRUE(r1_state.finished);
+}
+
+IN_PROC_BROWSER_TEST_F(ContentSettingBubbleModelQuietRequestTest,
+ PreemptionClosesOpenBubble) {
+ content::WebContents* web_contents = OpenTab();
+
+ MockRequestState r1_state;
+ AddQuietRequest(web_contents, permissions::RequestType::kNotifications,
+ QuietUiReason::kTriggeredDueToAbusiveRequests, &r1_state);
+
+ auto bubble = CreateBubble(web_contents);
+ auto owner = FakeOwner::Create(*bubble, 0);
+ EXPECT_FALSE(owner->was_closed());
+
+ MockRequestState r2_state;
+ AddQuietRequest(web_contents, permissions::RequestType::kGeolocation,
+ QuietUiReason::kEnabledInPrefs, &r2_state);
+
+ EXPECT_TRUE(owner->was_closed());
+}
+
+IN_PROC_BROWSER_TEST_F(ContentSettingBubbleModelQuietRequestTest,
+ FinalizationClosesOpenBubble) {
+ content::WebContents* web_contents = OpenTab();
+
+ MockRequestState r1_state;
+ AddQuietRequest(web_contents, permissions::RequestType::kNotifications,
+ QuietUiReason::kTriggeredDueToAbusiveRequests, &r1_state);
+
+ auto bubble = CreateBubble(web_contents);
+ auto owner = FakeOwner::Create(*bubble, 0);
+ EXPECT_FALSE(owner->was_closed());
+
+ GetManager(web_contents)->Accept(/*prompt_options=*/std::monostate());
+ base::RunLoop().RunUntilIdle();
+
+ EXPECT_TRUE(owner->was_closed());
+}
diff --git a/content/browser/permissions/embedded_permission_control_checker_unittest.cc b/content/browser/permissions/embedded_permission_control_checker_unittest.cc
index 494fee9..77c1823 100644
--- a/content/browser/permissions/embedded_permission_control_checker_unittest.cc
+++ b/content/browser/permissions/embedded_permission_control_checker_unittest.cc
@@ -5,6 +5,7 @@
#include "content/browser/permissions/embedded_permission_control_checker.h"
#include <optional>
+#include <utility>
#include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
@@ -159,13 +160,17 @@
IgnoreRegisteregisterPageEmbeddedPermissionCheck) {
base::test::ScopedFeatureList features;
features.InitAndEnableFeature(blink::features::kBypassPepcSecurityForTesting);
- for (PermissionName name :
- {PermissionName::AUDIO_CAPTURE, PermissionName::VIDEO_CAPTURE,
- PermissionName::GEOLOCATION}) {
+ for (const auto& [name, source] :
+ {std::make_pair(PermissionName::AUDIO_CAPTURE,
+ CapabilityElementSource::kUserMedia),
+ std::make_pair(PermissionName::VIDEO_CAPTURE,
+ CapabilityElementSource::kUserMedia),
+ std::make_pair(PermissionName::GEOLOCATION,
+ CapabilityElementSource::kGeolocation)}) {
std::vector<std::unique_ptr<MockEmbeddedPermissionControlClient>> clients(
kMaxPEPCPerPage + 3);
for (size_t i = 0; i < kMaxPEPCPerPage + 3; ++i) {
- clients[i] = CreateEmbeddedPermissionControlClient({name});
+ clients[i] = CreateEmbeddedPermissionControlClient({name}, source);
clients[i]->ExpectEmbeddedPermissionControlRegistered();
}
}
@@ -173,19 +178,25 @@
TEST_F(EmbeddedPermissionControlCheckerTest,
RegisterPageEmbeddedPermissionSinglePermission) {
- for (PermissionName name :
- {PermissionName::AUDIO_CAPTURE, PermissionName::VIDEO_CAPTURE,
- PermissionName::GEOLOCATION}) {
+ for (const auto& [name, source] :
+ {std::make_pair(PermissionName::AUDIO_CAPTURE,
+ CapabilityElementSource::kUserMedia),
+ std::make_pair(PermissionName::VIDEO_CAPTURE,
+ CapabilityElementSource::kUserMedia),
+ std::make_pair(PermissionName::GEOLOCATION,
+ CapabilityElementSource::kGeolocation)}) {
std::vector<std::unique_ptr<MockEmbeddedPermissionControlClient>> clients(
kMaxPEPCPerPage);
for (size_t i = 0; i < kMaxPEPCPerPage; ++i) {
- clients[i] = CreateEmbeddedPermissionControlClient({name});
+ clients[i] = CreateEmbeddedPermissionControlClient({name}, source);
clients[i]->ExpectEmbeddedPermissionControlRegistered();
}
- auto pending_client_1 = CreateEmbeddedPermissionControlClient({name});
+ auto pending_client_1 =
+ CreateEmbeddedPermissionControlClient({name}, source);
pending_client_1->ExpectEmbeddedPermissionControlNotRegistered();
- auto pending_client_2 = CreateEmbeddedPermissionControlClient({name});
+ auto pending_client_2 =
+ CreateEmbeddedPermissionControlClient({name}, source);
pending_client_2->ExpectEmbeddedPermissionControlNotRegistered();
clients.pop_back();
pending_client_1->ExpectEmbeddedPermissionControlRegistered();
@@ -207,13 +218,17 @@
// Embedded permission control of a single permission will not count towards
// the grouped one.
- for (PermissionName name :
- {PermissionName::AUDIO_CAPTURE, PermissionName::VIDEO_CAPTURE,
- PermissionName::GEOLOCATION}) {
+ for (const auto& [name, source] :
+ {std::make_pair(PermissionName::AUDIO_CAPTURE,
+ CapabilityElementSource::kUserMedia),
+ std::make_pair(PermissionName::VIDEO_CAPTURE,
+ CapabilityElementSource::kUserMedia),
+ std::make_pair(PermissionName::GEOLOCATION,
+ CapabilityElementSource::kGeolocation)}) {
std::vector<std::unique_ptr<MockEmbeddedPermissionControlClient>> clients(
kMaxPEPCPerPage);
for (size_t i = 0; i < kMaxPEPCPerPage; ++i) {
- clients[i] = CreateEmbeddedPermissionControlClient({name});
+ clients[i] = CreateEmbeddedPermissionControlClient({name}, source);
clients[i]->ExpectEmbeddedPermissionControlRegistered();
}
}
@@ -246,13 +261,13 @@
TEST_F(GeolocationEmbeddedPermissionControlCheckerTest,
DecouplePermissionSources) {
- // Register `kMaxPEPCPerPage` clients for the permission element source.
+ // Register `kMaxPEPCPerPage` clients for the user-media element source.
std::vector<std::unique_ptr<MockEmbeddedPermissionControlClient>>
- permission_clients(kMaxPEPCPerPage);
+ user_media_clients(kMaxPEPCPerPage);
for (size_t i = 0; i < kMaxPEPCPerPage; ++i) {
- permission_clients[i] = CreateEmbeddedPermissionControlClient(
- {PermissionName::GEOLOCATION}, CapabilityElementSource::kUserMedia);
- permission_clients[i]->ExpectEmbeddedPermissionControlRegistered();
+ user_media_clients[i] = CreateEmbeddedPermissionControlClient(
+ {PermissionName::AUDIO_CAPTURE}, CapabilityElementSource::kUserMedia);
+ user_media_clients[i]->ExpectEmbeddedPermissionControlRegistered();
}
// Register `kMaxPEPCPerPage` clients for the geolocation element source.
@@ -266,18 +281,18 @@
}
// Create one more client for each source, which should not be registered yet.
- auto pending_permission_client = CreateEmbeddedPermissionControlClient(
... (truncated)
Loading diff…
Original Bug Report
reported by [email protected]
Potential Permission Decision Spoofing in PWA Quiet Permission Bubble
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 error in the quiet permission UI model allows for permission-decision spoofing in installed PWAs. When a new permission request preempts an existing one, the PWA’s stale permission bubble remains open, causing user interactions with it to be applied to the new, unrelated request.
Affected files:
chrome/browser/ui/content_settings/content_setting_bubble_model.ccchrome/browser/ui/views/location_bar/content_setting_image_view.cccomponents/permissions/permission_request_manager.ccchrome/browser/ui/views/permissions/permission_prompt_factory.ccchrome/browser/ui/views/permissions/permission_prompt_quiet_icon.ccchrome/browser/ui/views/content_setting_bubble_contents.cc
Estimated timestamp from git blame: 2021-08-09
Summary
A logic error in ContentSettingQuietRequestBubbleModel::OnDoneButtonClicked combined with a lifecycle management issue in PWA windows allows for potential permission-decision spoofing. Specifically, the bubble’s action is computed by re-reading the PermissionRequestManager state at the moment of a click, rather than acting on the specific request the bubble was created for. In PWA windows, unlike standard browser tabs, this bubble is not automatically closed when the underlying permission request is swapped or preempted.
Root Cause
The vulnerability resides in ContentSettingQuietRequestBubbleModel::OnDoneButtonClicked (found in chrome/browser/ui/content_settings/content_setting_bubble_model.cc). This method re-evaluates the active permission request and its ‘quiet UI reason’ at the time of the button click:
void ContentSettingQuietRequestBubbleModel::OnDoneButtonClicked() {
permissions::PermissionRequestManager* manager =
permissions::PermissionRequestManager::FromWebContents(web_contents());
// ...
auto quiet_ui_reason = manager->ReasonForUsingQuietUi();
const permissions::PermissionRequest& request = *manager->Requests()[0];
// ...
switch (*quiet_ui_reason) {
case QuietUiReason::kEnabledInPrefs:
// ... other reasons ...
manager->Accept(/*prompt_options=*/std::monostate());
break;
case QuietUiReason::kTriggeredDueToAbusiveRequests:
// ... other abusive reasons ...
manager->Deny(/*prompt_options=*/std::monostate());
break;
}
}
While the button labels are configured during construction based on the initial request (e.g., “Continue blocking” for abusive notification requests), the final action depends on whatever request is active in the PermissionRequestManager (PRM) when the button is clicked.
In standard browser tabs, the ChipController observes the PRM and explicitly closes the permission bubble if the request is finalized or preempted. However, in an installed PWA window, the UI follows a different path through PermissionPromptQuietIcon and ContentSettingImageView. This path lacks a PRM observer to close the bubble when the underlying request changes. Consequently, if a new permission request is made while the quiet UI bubble is open, the PRM swaps the requests, but the stale bubble remains visible.
Potential Attack Scenario
- Preparation: A user installs a PWA from an attacker’s origin. The origin is flagged for abusive notifications, or the user has enabled quiet permission prompts in settings.
- Triggering R1: The PWA requests notification permissions. A quiet UI icon appears in the PWA toolbar.
- User Interaction: The user clicks the icon, opening a bubble. The bubble’s “Done” button is labeled “Continue blocking” (due to the abusive notification status).
- Request Swap (R2): Before the user clicks, the page triggers a second permission request (e.g., Geolocation) that also qualifies for quiet UI (e.g., via prediction services or lack of user gesture).
- State Change: The PRM preempts the notification request (R1) with the geolocation request (R2). The stale bubble remains open in the PWA window, and its anchor icon may be hidden or replaced.
- Spoofing Execution: The user clicks “Continue blocking”.
OnDoneButtonClicked()re-reads the PRM state, finds the Geolocation request (R2), and callsmanager->Accept()because the reason for R2 (e.g., lack of gesture) is handled via theAccept()path.
As a result, Geolocation is granted to the origin despite the user intending to “Continue blocking” a notification request.
Suggested Fix
- The
ContentSettingQuietRequestBubbleModelshould store the unique ID or the specificRequestTypeandQuietUiReasonit was initialized with and verify that the current request in thePermissionRequestManagerstill matches these parameters before taking an action. - Alternatively, the PWA UI components (such as
WebAppContentSettingsContainerorContentSettingImageView) should observe thePermissionRequestManagerand ensure any open bubbles are closed when the active request is preempted or finalized, similar to theChipControllerimplementation.
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