CVE-2026-87507
Overview
Background
- Download Bubble security subpage
- the
DownloadBubbleSecurityViewsurface that shows a dangerous-download warning with action buttons (e.g.DownloadCommands::Command::KEEP/DownloadCommands::Command::DISCARD) wired onto the dialog’s OK/Cancel buttons. - Picture-in-picture occlusion tracking
- a mechanism (
PictureInPictureOcclusionObserver/ScopedPictureInPictureOcclusionObservation/PictureInPictureOcclusionTracker) that notifies a widget when an always-on-top picture-in-picture window is covering it, so covered controls can be disabled to prevent click-through. - Input protection
- a timer (
TriggerInputProtection()) that briefly ignores clicks after a UI change to defeat clickjacking / unintended clicks. - `DialogDelegate::SetButtonEnabled`
- the API used to enable/disable the dialog OK/Cancel buttons (
bubble_delegate_->SetButtonEnabled(button_type, ...)).
Root Cause Analysis
An earlier change added picture-in-picture occlusion tracking to the download bubble but, per the commit message, only disabled the row-view controls; the dialog OK/Cancel buttons that back the DownloadBubbleSecurityView subpage were left enabled while the bubble was occluded. In the button-population path (the function containing the @@ -537 hunk, named UpdateButton in the commit message but not shown in the hunk header — (inferred) from the header alone), the kCancel branch unconditionally called bubble_delegate_->SetButtonEnabled(button_type, true), so the security-decision buttons stayed clickable even when a picture-in-picture window covered the warning. The invariant violated is that a control the user cannot see (because it is occluded) must not remain actionable.
The fix makes DownloadBubbleSecurityView a PictureInPictureOcclusionObserver, observes the bubble widget in AddedToWidget(), tracks state in occluded_, and in OnOcclusionStateChanged disables/re-enables the dialog buttons (via UpdateButtons()) while also calling TriggerInputProtection() on the occluded→un-occluded transition. It also replaces the unconditional enable with bubble_delegate_->SetButtonEnabled(button_type, !occluded_) so that re-populating buttons while occluded does not re-enable them. This is purely the addition of an occlusion guard on button enablement; the diff adds no new download-handling logic.
occluded_ state.Attack Path
- Surface the warning subpage
A download that triggers a security warning causes Chrome to show the
DownloadBubbleSecurityViewsubpage with dialog buttons (e.g.KEEP/DISCARD) wired onto the bubble’s OK/Cancel buttons. - Occlude the bubble A picture-in-picture window is placed over the download bubble so the warning text and buttons are visually covered while the widget remains live.
- Click through the hidden buttons
Because the pre-patch
kCancelpath unconditionally calledSetButtonEnabled(button_type, true)and no occlusion observer disabled the dialog buttons, the covered OK/Cancel buttons stay enabled and remain clickable, so a click intended for the picture-in-picture content (or an induced click) actuates the hidden security decision without the user seeing the warning — a clickjacking / UI-misrepresentation condition. - Escalation
Actuating a hidden
KEEP(or equivalent) accepts a download the user never knowingly approved.(inferred)— the diff only adds the enablement guard and does not itself demonstrate what accepting the download leads to.
Impact Assessment
(inferred), and the advisory reports neither in-the-wild exploitation nor a severity to restate here.Changed Functions
| Function | Change | Notes |
|---|---|---|
ifchrome/browser/ui/views/download/bubble/download_bubble_security_view.cc |
modified | |
DownloadBubbleSecurityViewchrome/browser/ui/views/download/bubble/download_bubble_security_view.h |
modified | |
TEST_Fchrome/browser/ui/views/download/bubble/download_bubble_security_view_unittest.cc |
modified |
Files Changed
chrome/browser/ui/views/download/bubble/download_bubble_security_view.ccchrome/browser/ui/views/download/bubble/download_bubble_security_view.hchrome/browser/ui/views/download/bubble/download_bubble_security_view_unittest.cc
Audit Directions
- Partial occlusion coverageWhen an occlusion/visibility guard is added for one set of controls (here the row-view), check every other actionable surface on the same widget (dialog OK/Cancel buttons, context menus, subpages) that was left enabled.
- Unconditional re-enable on re-populateLook for code that unconditionally sets a control enabled (
SetButtonEnabled(..., true)) during (re)initialization without consulting a current occlusion/protection state, which silently undoes a disable. - Security-decision UI without input protectionAudit dialogs that carry consequential choices (keep/discard, allow/block) for missing
TriggerInputProtection()on visibility transitions and for observers of always-on-top overlays such as picture-in-picture.
Patch
From 6d86d50a2c19937cba6949c28dd2951b6ccc665a Mon Sep 17 00:00:00 2001
From: Yaw Frempong <yawfrempong@google.com>
Date: Mon, 10 Aug 2026 21:44:42 -0700
Subject: [PATCH] [Download Bubble] Disable subpage dialog buttons when occluded
DownloadBubbleSecurityView wires its primary/secondary subpage buttons
onto the bubble's DialogClientView OK/Cancel buttons. The earlier change
that added picture-in-picture occlusion tracking to the download bubble
only disabled the row-view controls; the dialog buttons used by the
subpage were left enabled while the bubble is occluded.
Mirror the row-view pattern: make DownloadBubbleSecurityView a
PictureInPictureOcclusionObserver, observe the bubble widget in
AddedToWidget(), and disable the dialog OK/Cancel buttons via
DialogDelegate::SetButtonEnabled() while occluded. Also consult the
occlusion state in UpdateButton() so re-populating the buttons does not
re-enable them.
Reviewed in https://crrev.com/i/9596093
Bug: 514009699
Change-Id: I7b01408059e9de9c8cc26bb0fa2f1e1a6c8e91f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8170820
Reviewed-by: Brian Lefler <bcl@google.com>
Reviewed-by: Lily Chen <chlily@chromium.org>
Commit-Queue: Yaw Frempong <yawfrempong@google.com>
Cr-Commit-Position: refs/heads/main@{#1676911}
---
diff --git a/chrome/browser/ui/views/download/bubble/download_bubble_security_view.cc b/chrome/browser/ui/views/download/bubble/download_bubble_security_view.cc
index 6270c8d..0853bdda 100644
--- a/chrome/browser/ui/views/download/bubble/download_bubble_security_view.cc
+++ b/chrome/browser/ui/views/download/bubble/download_bubble_security_view.cc
@@ -537,9 +537,10 @@
&HandleButtonClickWithDefaultClose, weak_factory_.GetWeakPtr(),
button_info.command, is_secondary_button));
+ bubble_delegate_->SetButtonEnabled(button_type, !occluded_);
+
if (button_type == ui::mojom::DialogButton::kCancel) {
bubble_delegate_->SetCancelCallbackWithClose(callback);
- bubble_delegate_->SetButtonEnabled(button_type, true);
views::LabelButton* button = bubble_delegate_->GetCancelButton();
if (button_info.text_color) {
button->SetEnabledTextColors(*button_info.text_color);
@@ -688,6 +689,25 @@
DownloadBubbleSecurityView::~DownloadBubbleSecurityView() = default;
+void DownloadBubbleSecurityView::AddedToWidget() {
+ views::Widget* widget = GetWidget();
+ pip_occlusion_observation_.Observe(widget);
+}
+
+void DownloadBubbleSecurityView::OnOcclusionStateChanged(bool occluded) {
+ if (occluded_ == occluded) {
+ return;
+ }
+
+ // If transitioning from occluded to un-occluded, restart the input protection
+ // timer to prevent clickjacking/unintended clicks.
+ if (occluded_ && !occluded && bubble_delegate_) {
+ bubble_delegate_->TriggerInputProtection();
+ }
+ occluded_ = occluded;
+ UpdateButtons();
+}
+
int DownloadBubbleSecurityView::GetMinimumBubbleWidth() const {
return ChromeLayoutProvider::Get()->GetSnappedDialogWidth(
bubble_delegate_->GetDialogClientView()->GetMinimumSize().width());
diff --git a/chrome/browser/ui/views/download/bubble/download_bubble_security_view.h b/chrome/browser/ui/views/download/bubble/download_bubble_security_view.h
index 65b4902..62ffc349 100644
--- a/chrome/browser/ui/views/download/bubble/download_bubble_security_view.h
+++ b/chrome/browser/ui/views/download/bubble/download_bubble_security_view.h
@@ -14,6 +14,8 @@
#include "base/types/optional_ref.h"
#include "chrome/browser/download/download_item_warning_data.h"
#include "chrome/browser/download/download_ui_model.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_observer.h"
+#include "chrome/browser/picture_in_picture/scoped_picture_in_picture_occlusion_observation.h"
#include "chrome/browser/ui/download/download_bubble_security_view_info.h"
#include "components/download/public/common/download_danger_type.h"
#include "components/offline_items_collection/core/offline_item.h"
@@ -34,6 +36,7 @@
class DownloadBubbleSecurityView
: public views::View,
+ public PictureInPictureOcclusionObserver,
public DownloadBubbleSecurityViewInfoObserver {
METADATA_HEADER(DownloadBubbleSecurityView, views::View)
@@ -90,6 +93,12 @@
delete;
~DownloadBubbleSecurityView() override;
+ // views::View:
+ void AddedToWidget() override;
+
+ // PictureInPictureOcclusionObserver:
+ void OnOcclusionStateChanged(bool occluded) override;
+
// Whether this view is properly associated with a download. The rest of the
// public method calls on this view do not make sense if not initialized.
bool IsInitialized() const;
@@ -184,6 +193,12 @@
// double-logging.
bool did_log_action_ = false;
+ // Whether the bubble widget is currently occluded by a picture-in-picture
+ // window. The dialog buttons are disabled while occluded.
+ bool occluded_ = false;
+
+ ScopedPictureInPictureOcclusionObservation pip_occlusion_observation_{this};
+
base::WeakPtrFactory<DownloadBubbleSecurityView> weak_factory_{this};
};
diff --git a/chrome/browser/ui/views/download/bubble/download_bubble_security_view_unittest.cc b/chrome/browser/ui/views/download/bubble/download_bubble_security_view_unittest.cc
index c1ecf56..6553966 100644
--- a/chrome/browser/ui/views/download/bubble/download_bubble_security_view_unittest.cc
+++ b/chrome/browser/ui/views/download/bubble/download_bubble_security_view_unittest.cc
@@ -14,6 +14,8 @@
#include "chrome/browser/download/download_ui_model.h"
#include "chrome/browser/download/mock_download_core_service.h"
#include "chrome/browser/download/offline_item_utils.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_tracker.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
#include "chrome/browser/ui/download/download_bubble_security_view_info.h"
#include "chrome/browser/ui/views/chrome_layout_provider.h"
#include "chrome/browser/ui/views/download/bubble/download_bubble_contents_view.h"
@@ -31,6 +33,7 @@
#include "ui/base/mojom/dialog_button.mojom.h"
#include "ui/color/color_id.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h"
+#include "ui/views/controls/button/md_text_button.h"
#include "ui/views/view.h"
#include "ui/views/window/dialog_client_view.h"
@@ -166,15 +169,17 @@
bubble_delegate_ = bubble_delegate.get();
bubble_navigator_ = std::make_unique<MockDownloadBubbleNavigationHandler>(
*security_view_info_);
- views::BubbleDialogDelegate::CreateBubbleDeprecated(
- std::move(bubble_delegate),
- views::Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET);
- bubble_delegate_->GetWidget()->Show();
+
security_view_ = bubble_delegate_->SetContentsView(
std::make_unique<DownloadBubbleSecurityView>(
security_view_delegate_.get(), *security_view_info_,
bubble_navigator_->GetWeakPtr(), bubble_delegate_));
+ views::BubbleDialogDelegate::CreateBubbleDeprecated(
+ std::move(bubble_delegate),
+ views::Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET);
+ bubble_delegate_->GetWidget()->Show();
+
DownloadCoreServiceFactory::GetInstance()->SetTestingFactory(
profile_, base::BindRepeating(&BuildMockDownloadCoreService));
MockDownloadCoreService* mock_dcs = static_cast<MockDownloadCoreService*>(
@@ -315,6 +320,46 @@
static_cast<int>(ui::mojom::DialogButton::kNone));
}
+TEST_F(DownloadBubbleSecurityViewTest,
+ DialogButtonsDisabledWhenOccludedByPictureInPicture) {
+ security_view_info_->InitializeForDownload(*row1_model_);
+ security_view_info_->SetSubpageButtonsForTesting(
+ {SubpageButton(DownloadCommands::Command::DISCARD, std::u16string(),
+ /*is_prominent=*/true),
+ SubpageButton(DownloadCommands::Command::KEEP, std::u16string(),
+ /*is_prominent=*/false, ui::kColorAlertHighSeverity)});
+
+ ASSERT_NE(nullptr, bubble_delegate_->GetOkButton());
+ ASSERT_NE(nullptr, bubble_delegate_->GetCancelButton());
+ EXPECT_TRUE(bubble_delegate_->GetOkButton()->GetEnabled());
+ EXPECT_TRUE(bubble_delegate_->GetCancelButton()->GetEnabled());
+
+ PictureInPictureOcclusionTracker* tracker =
+ PictureInPictureWindowManager::GetInstance()->GetOcclusionTracker();
+ ASSERT_NE(nullptr, tracker);
+ tracker->SetWidgetOcclusionStateForTesting(bubble_delegate_->GetWidget(),
+ /*occluded=*/true);
+
+ EXPECT_FALSE(bubble_delegate_->GetOkButton()->GetEnabled());
+ EXPECT_FALSE(bubble_delegate_->GetCancelButton()->GetEnabled());
+
+ // Re-initializing the view while occluded must not re-enable the buttons.
+ security_view_info_->InitializeForDownload(*row1_model_);
+ security_view_info_->SetSubpageButtonsForTesting(
+ {SubpageButton(DownloadCommands::Command::DISCARD, std::u16string(),
+ /*is_prominent=*/true),
+ SubpageButton(DownloadCommands::Command::KEEP, std::u16string(),
+ /*is_prominent=*/false, ui::kColorAlertHighSeverity)});
+ EXPECT_FALSE(bubble_delegate_->GetOkButton()->GetEnabled());
+ EXPECT_FALSE(bubble_delegate_->GetCancelButton()->GetEnabled());
+
+ tracker->SetWidgetOcclusionStateForTesting(bubble_delegate_->GetWidget(),
+ /*occluded=*/false);
+
+ EXPECT_TRUE(bubble_delegate_->GetOkButton()->GetEnabled());
+ EXPECT_TRUE(bubble_delegate_->GetCancelButton()->GetEnabled());
+}
+
TEST_F(DownloadBubbleSecurityViewTest, VerifyLogWarningActions) {
DownloadItemWarningData::AddWarningActionEvent(
&download_item1_, WarningSurface::BUBBLE_MAINPAGE, WarningAction::SHOWN);
@@ -501,7 +546,8 @@
security_view_info_->InitializeForDownload(*row1_model_);
security_view_info_->SetSubpageButtonsForTesting({SubpageButton(
DownloadCommands::Command::DISCARD,
- std::u16string(u"really really really really really really long "
+ std::u16string(u"really really really really really really really "
+ u"really really really really really really long "
u"button text"),
/*is_prominent=*/true)});
UpdateView();
Regression Test / PoC
diff --git a/chrome/browser/ui/views/download/bubble/download_bubble_security_view_unittest.cc b/chrome/browser/ui/views/download/bubble/download_bubble_security_view_unittest.cc
index c1ecf56..6553966 100644
--- a/chrome/browser/ui/views/download/bubble/download_bubble_security_view_unittest.cc
+++ b/chrome/browser/ui/views/download/bubble/download_bubble_security_view_unittest.cc
@@ -14,6 +14,8 @@
#include "chrome/browser/download/download_ui_model.h"
#include "chrome/browser/download/mock_download_core_service.h"
#include "chrome/browser/download/offline_item_utils.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_tracker.h"
+#include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h"
#include "chrome/browser/ui/download/download_bubble_security_view_info.h"
#include "chrome/browser/ui/views/chrome_layout_provider.h"
#include "chrome/browser/ui/views/download/bubble/download_bubble_contents_view.h"
@@ -31,6 +33,7 @@
#include "ui/base/mojom/dialog_button.mojom.h"
#include "ui/color/color_id.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h"
+#include "ui/views/controls/button/md_text_button.h"
#include "ui/views/view.h"
#include "ui/views/window/dialog_client_view.h"
@@ -166,15 +169,17 @@
bubble_delegate_ = bubble_delegate.get();
bubble_navigator_ = std::make_unique<MockDownloadBubbleNavigationHandler>(
*security_view_info_);
- views::BubbleDialogDelegate::CreateBubbleDeprecated(
- std::move(bubble_delegate),
- views::Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET);
- bubble_delegate_->GetWidget()->Show();
+
security_view_ = bubble_delegate_->SetContentsView(
std::make_unique<DownloadBubbleSecurityView>(
security_view_delegate_.get(), *security_view_info_,
bubble_navigator_->GetWeakPtr(), bubble_delegate_));
+ views::BubbleDialogDelegate::CreateBubbleDeprecated(
+ std::move(bubble_delegate),
+ views::Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET);
+ bubble_delegate_->GetWidget()->Show();
+
DownloadCoreServiceFactory::GetInstance()->SetTestingFactory(
profile_, base::BindRepeating(&BuildMockDownloadCoreService));
MockDownloadCoreService* mock_dcs = static_cast<MockDownloadCoreService*>(
@@ -315,6 +320,46 @@
static_cast<int>(ui::mojom::DialogButton::kNone));
}
+TEST_F(DownloadBubbleSecurityViewTest,
+ DialogButtonsDisabledWhenOccludedByPictureInPicture) {
+ security_view_info_->InitializeForDownload(*row1_model_);
+ security_view_info_->SetSubpageButtonsForTesting(
+ {SubpageButton(DownloadCommands::Command::DISCARD, std::u16string(),
+ /*is_prominent=*/true),
+ SubpageButton(DownloadCommands::Command::KEEP, std::u16string(),
+ /*is_prominent=*/false, ui::kColorAlertHighSeverity)});
+
+ ASSERT_NE(nullptr, bubble_delegate_->GetOkButton());
+ ASSERT_NE(nullptr, bubble_delegate_->GetCancelButton());
+ EXPECT_TRUE(bubble_delegate_->GetOkButton()->GetEnabled());
+ EXPECT_TRUE(bubble_delegate_->GetCancelButton()->GetEnabled());
+
+ PictureInPictureOcclusionTracker* tracker =
+ PictureInPictureWindowManager::GetInstance()->GetOcclusionTracker();
+ ASSERT_NE(nullptr, tracker);
+ tracker->SetWidgetOcclusionStateForTesting(bubble_delegate_->GetWidget(),
+ /*occluded=*/true);
+
+ EXPECT_FALSE(bubble_delegate_->GetOkButton()->GetEnabled());
+ EXPECT_FALSE(bubble_delegate_->GetCancelButton()->GetEnabled());
+
+ // Re-initializing the view while occluded must not re-enable the buttons.
+ security_view_info_->InitializeForDownload(*row1_model_);
+ security_view_info_->SetSubpageButtonsForTesting(
+ {SubpageButton(DownloadCommands::Command::DISCARD, std::u16string(),
+ /*is_prominent=*/true),
+ SubpageButton(DownloadCommands::Command::KEEP, std::u16string(),
+ /*is_prominent=*/false, ui::kColorAlertHighSeverity)});
+ EXPECT_FALSE(bubble_delegate_->GetOkButton()->GetEnabled());
+ EXPECT_FALSE(bubble_delegate_->GetCancelButton()->GetEnabled());
+
+ tracker->SetWidgetOcclusionStateForTesting(bubble_delegate_->GetWidget(),
+ /*occluded=*/false);
+
+ EXPECT_TRUE(bubble_delegate_->GetOkButton()->GetEnabled());
+ EXPECT_TRUE(bubble_delegate_->GetCancelButton()->GetEnabled());
+}
+
TEST_F(DownloadBubbleSecurityViewTest, VerifyLogWarningActions) {
DownloadItemWarningData::AddWarningActionEvent(
&download_item1_, WarningSurface::BUBBLE_MAINPAGE, WarningAction::SHOWN);
@@ -501,7 +546,8 @@
security_view_info_->InitializeForDownload(*row1_model_);
security_view_info_->SetSubpageButtonsForTesting({SubpageButton(
DownloadCommands::Command::DISCARD,
- std::u16string(u"really really really really really really long "
+ std::u16string(u"really really really really really really really "
+ u"really really really really really really long "
u"button text"),
/*is_prominent=*/true)});
UpdateView();