Medium chrome Memory Corruption 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactMemory corruption in Interstitials
DescriptionMemory corruption in Interstitials
ComponentInterstitials
Sub-componentUI>Browser>Interstitials
Bug ClassMemory Corruption
Tracker553155133
Fix commitb1df05cbd74f (chromium/src) +328/-12
CISA KEVNot listed
CreditedGoogle
Disclosed2026-10-20

Changed Functions

FunctionChangeNotes
BubbleDialogModelHostTestPassKey
ui/views/bubble/bubble_dialog_model_host_unittest.cc
modified

Files Changed

  • ui/views/bubble/bubble_dialog_model_host.h
  • chrome/browser/ssl/ask_before_http_dialog_controller_interactive_uitest.cc
  • ui/views/bubble/bubble_dialog_model_host.cc
  • ui/views/bubble/bubble_dialog_model_host_unittest.cc
  • chrome/browser/devtools/devtools_connection_dialog_browsertest.cc
  • chrome/browser/ui/views/scheduled_restart/scheduled_restart_bubble_view_browsertest.cc
From b1df05cbd74f9e8621eb2a1b5f9dd932eadfc8d7 Mon Sep 17 00:00:00 2001
From: Victor Miura <vmiura@chromium.org>
Date: Tue, 22 Sep 2026 14:28:57 -0700
Subject: [PATCH] Apply dialog input protection to DialogModel extra buttons

BubbleDialogModelHost wired the extra button and the extra link
directly to the dialog model callback, so they were not covered
by the input protection that DialogClientView::ButtonPressed()
applies to the ok and cancel buttons. A pointer event arriving
within the protection window of the dialog becoming visible, or
arriving in rapid succession, could activate the extra button
even though the same event on the ok or cancel button would be
ignored as a possibly unintended interaction.

Route extra button presses and extra link clicks through the
dialog's InputEventActivationProtector via
DialogClientView::IsPossiblyUnintendedInteraction(), using the
same key event allowance as the standard buttons. This gives
extra views, such as the Ask-before-HTTP dialog's "Continue to
site" button, the same activation protection and protection
lifecycle as the ok and cancel buttons.

Add regression tests covering pointer and key events on extra
buttons and extra links, plus an end-to-end test for the
Ask-before-HTTP dialog, and adjust the DevTools connection
dialog test to click after the protection window has passed.

TAG=agy
CONV=3ef32ea6-2729-4f27-8412-eb9507018261

Bug: 553155133
Change-Id: I034607b8b89b75dedbe408b447f93525f345b5ef
Fixed: 553155133
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8433801
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Auto-Submit: Victor Miura <vmiura@chromium.org>
Commit-Queue: Victor Miura <vmiura@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1702965}
---
diff --git a/ui/views/bubble/bubble_dialog_model_host.cc b/ui/views/bubble/bubble_dialog_model_host.cc
index cb78c64..311c2aa 100644
--- a/ui/views/bubble/bubble_dialog_model_host.cc
+++ b/ui/views/bubble/bubble_dialog_model_host.cc
@@ -19,6 +19,7 @@
 #include "ui/base/mojom/dialog_button.mojom.h"
 #include "ui/base/mojom/ui_base_types.mojom-shared.h"
 #include "ui/base/ui_base_types.h"
+#include "ui/events/event.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/strings/grit/ui_strings.h"
@@ -32,6 +33,7 @@
 #include "ui/views/controls/combobox/combobox.h"
 #include "ui/views/controls/editable_combobox/editable_password_combobox.h"
 #include "ui/views/controls/label.h"
+#include "ui/views/controls/link.h"
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/controls/separator.h"
 #include "ui/views/controls/styled_label.h"
@@ -44,6 +46,7 @@
 #include "ui/views/metadata/view_factory.h"
 #include "ui/views/style/typography.h"
 #include "ui/views/view_class_properties.h"
+#include "ui/views/window/dialog_client_view.h"
 
 namespace views {
 namespace {
@@ -937,12 +940,11 @@
   if (ui::DialogModel::Button* extra_button =
           model_->extra_button(DialogModelHost::GetPassKey())) {
     DCHECK(!model_->extra_link(DialogModelHost::GetPassKey()));
-    auto builder =
-        views::Builder<MdTextButton>()
-            .SetCallback(base::BindRepeating(
-                &ui::DialogModel::Button::OnPressed,
-                base::Unretained(extra_button), DialogModelHost::GetPassKey()))
-            .SetText(extra_button->label());
+    auto builder = views::Builder<MdTextButton>()
+                       .SetCallback(base::BindRepeating(
+                           &BubbleDialogModelHost::OnExtraButtonPressed,
+                           base::Unretained(this)))
+                       .SetText(extra_button->label());
     if (extra_button->style()) {
       builder.SetStyle(extra_button->style().value());
     }
@@ -952,7 +954,8 @@
                  model_->extra_link(DialogModelHost::GetPassKey())) {
     DCHECK(extra_link->callback().has_value());
     auto link = std::make_unique<views::Link>(extra_link->text());
-    link->SetCallback(extra_link->callback().value());
+    link->SetCallback(base::BindRepeating(
+        &BubbleDialogModelHost::OnExtraLinkClicked, base::Unretained(this)));
     SetExtraView(std::move(link));
   }
 
@@ -1047,6 +1050,28 @@
              : true;
 }
 
+void BubbleDialogModelHost::OnExtraButtonPressed(const ui::Event& event) {
+  if (!model_ || IsPossiblyUnintendedInteraction(event)) {
+    return;
+  }
+  model_->extra_button(DialogModelHost::GetPassKey())
+      ->OnPressed(DialogModelHost::GetPassKey(), event);
+}
+
+void BubbleDialogModelHost::OnExtraLinkClicked(const ui::Event& event) {
+  if (!model_ || IsPossiblyUnintendedInteraction(event)) {
+    return;
+  }
+  model_->extra_link(DialogModelHost::GetPassKey())->callback()->Run(event);
+}
+
+bool BubbleDialogModelHost::IsPossiblyUnintendedInteraction(
+    const ui::Event& event) {
+  DialogClientView* const client_view = GetDialogClientView();
+  return client_view && client_view->IsPossiblyUnintendedInteraction(
+                            event, ShouldAllowKeyEventsDuringInputProtection());
+}
+
 BubbleDialogModelHost::~BubbleDialogModelHost() {
   // Detach ContentsView as it's referring to state that's about to be
   // destroyed.
diff --git a/ui/views/bubble/bubble_dialog_model_host.h b/ui/views/bubble/bubble_dialog_model_host.h
index a3db5844..b2f64eb7 100644
--- a/ui/views/bubble/bubble_dialog_model_host.h
+++ b/ui/views/bubble/bubble_dialog_model_host.h
@@ -168,6 +168,14 @@
 
   bool IsModalDialog() const;
 
+  // Handles presses of the extra button and extra link. Unlike the ok and
+  // cancel buttons, the extra view is not routed through
+  // DialogClientView::ButtonPressed(), so these apply the dialog's input
+  // protection before running the model callback.
+  void OnExtraButtonPressed(const ui::Event& event);
+  void OnExtraLinkClicked(const ui::Event& event);
+  bool IsPossiblyUnintendedInteraction(const ui::Event& event);
+
   std::unique_ptr<ui::DialogModel> model_;
   raw_ptr<BubbleDialogModelHostContentsView> contents_view_ = nullptr;
   base::CallbackListSubscription on_contents_changed_subscription_;
diff --git a/chrome/browser/devtools/devtools_connection_dialog_browsertest.cc b/chrome/browser/devtools/devtools_connection_dialog_browsertest.cc
index 9139fa93..459cbfc 100644
--- a/chrome/browser/devtools/devtools_connection_dialog_browsertest.cc
+++ b/chrome/browser/devtools/devtools_connection_dialog_browsertest.cc
@@ -15,6 +15,7 @@
 #include "ui/events/event.h"
 #include "ui/views/bubble/bubble_dialog_model_host.h"
 #include "ui/views/controls/button/md_text_button.h"
+#include "ui/views/metrics.h"
 #include "ui/views/test/button_test_api.h"
 #include "ui/views/test/widget_test.h"
 #include "ui/views/widget/widget.h"
@@ -100,12 +101,15 @@
 
   content::WebContentsAddedObserver new_tab_observer;
 
+  // The click timestamp must be past the dialog's input protection window,
+  // otherwise the extra button ignores it as a possibly unintended
+  // interaction.
   views::test::ButtonTestApi(
       static_cast<views::MdTextButton*>(dialog_model_dialog->GetExtraView()))
-      .NotifyClick(ui::MouseEvent(ui::EventType::kMousePressed, gfx::Point(),
-                                  gfx::Point(), ui::EventTimeForNow(),
-                                  ui::EF_LEFT_MOUSE_BUTTON,
-                                  ui::EF_LEFT_MOUSE_BUTTON));
+      .NotifyClick(ui::MouseEvent(
+          ui::EventType::kMousePressed, gfx::Point(), gfx::Point(),
+          ui::EventTimeForNow() + 2 * views::GetDoubleClickInterval(),
+          ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON));
 
   EXPECT_EQ(future.Get(),
             content::DevToolsManagerDelegate::AcceptConnectionResult::kDeny);
diff --git a/chrome/browser/ssl/ask_before_http_dialog_controller_interactive_uitest.cc b/chrome/browser/ssl/ask_before_http_dialog_controller_interactive_uitest.cc
index 0536cdc..960e55b 100644
--- a/chrome/browser/ssl/ask_before_http_dialog_controller_interactive_uitest.cc
+++ b/chrome/browser/ssl/ask_before_http_dialog_controller_interactive_uitest.cc
@@ -46,7 +46,15 @@
 #include "services/metrics/public/cpp/ukm_source.h"
 #include "ui/base/metadata/metadata_impl_macros.h"
 #include "ui/base/page_transition_types.h"
+#include "ui/events/base_event_utils.h"
+#include "ui/events/event.h"
+#include "ui/views/controls/button/button.h"
 #include "ui/views/controls/styled_label.h"
+#include "ui/views/interaction/interaction_test_util_views.h"
+#include "ui/views/metrics.h"
+#include "ui/views/test/button_test_api.h"
+#include "ui/views/window/dialog_client_view.h"
+#include "ui/views/window/dialog_delegate.h"
 
 using security_interstitials::MetricsHelper;
 using security_interstitials::https_only_mode::BlockingResult;
@@ -403,6 +411,68 @@
   ExpectUKMEntry(http_url, BlockingResult::kInterstitialProceed);
 }
 
+// Mouse clicks landing on the "Continue to site" button while the dialog's
+// input protection window is active (e.g. a click arriving right as the
+// dialog appears) must be ignored, matching the protection that the standard
+// dialog buttons get from DialogClientView. A click arriving after the
+// protection window has passed should still proceed to the site.
+IN_PROC_BROWSER_TEST_P(AskBeforeHttpDialogControllerUiTest,
+                       FailedUpgrade_WarningShown_EarlyContinueClickIgnored) {
+  DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kTestTab);
+
+  GURL http_url = http_server()->GetURL("bad-https.com", "/simple.html");
+
+  auto* contents = GetBrowser()->tab_strip_model()->GetActiveWebContents();
+  content::NavigationController::LoadURLParams params(http_url);
+  params.transition_type = ui::PAGE_TRANSITION_LINK;
+  content::NavigateToURLBlockUntilNavigationsComplete(contents, params, 1);
+  EXPECT_EQ(http_url, contents->GetLastCommittedURL());
+
+  RunTestSequence(
+      InAnyContext(
+          WaitForShow(AskBeforeHttpDialogController::kContinueButtonId)),
+      InSameContext(
+          InstrumentTab(kTestTab),
+          // Simulate a mouse click on the button within the input protection
+          // window. The protection window is restarted first so that the
+          // click reliably falls inside it, regardless of how much time has
+          // passed since the dialog was shown.
+          WithView(
+              AskBeforeHttpDialogController::kContinueButtonId,
+              [](views::Button* button) {
+                button->GetWidget()
+                    ->widget_delegate()
+                    ->AsDialogDelegate()
+                    ->GetDialogClientView()
+                    ->TriggerInputProtection(/*force_early=*/true);
+                views::test::InteractionTestUtilSimulatorViews::PressButton(
+                    button, ui::test::InteractionTestUtil::InputType::kMouse);
+              }),
+          // The click must have been ignored: the dialog is still showing.
+          EnsurePresent(AskBeforeHttpDialogController::kContinueButtonId),
+          // A click arriving after the protection window has passed is
+          // accepted and continues to the site.
+          WithView(
+              AskBeforeHttpDialogController::kContinueButtonId,
+              [](views::Button* button) {
+                const ui::MouseEvent click(
+                    ui::EventType::kMousePressed, gfx::PointF(), gfx::PointF(),
+                    ui::EventTimeForNow() + 2 * views::GetDoubleClickInterval(),
+                    ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON);
+                views::test::ButtonTestApi(button).NotifyClick(click);
+              })
+              .SetMustRemainVisible(false),
+          WaitForWebContentsNavigation(kTestTab, http_url)));
+
+  // Only the delayed click should count as a decision on the warning.
+  histograms()->ExpectBucketCount("interstitial.https_first_mode.decision",
+                                  MetricsHelper::Decision::SHOW, 1);
+  histograms()->ExpectBucketCount("interstitial.https_first_mode.decision",
+                                  MetricsHelper::Decision::PROCEED, 1);
+
+  ExpectUKMEntry(http_url, BlockingResult::kInterstitialProceed);
+}
+
 // If the user triggers an Ask-before-HTTP warning for a host and then
 // clicks the "Go back" button, they should navigate back to the previous
 // page (about:blank in this case).
diff --git a/chrome/browser/ui/views/scheduled_restart/scheduled_restart_bubble_view_browsertest.cc b/chrome/browser/ui/views/scheduled_restart/scheduled_restart_bubble_view_browsertest.cc
index 5d470fa3..581d009 100644
--- a/chrome/browser/ui/views/scheduled_restart/scheduled_restart_bubble_view_browsertest.cc
+++ b/chrome/browser/ui/views/scheduled_restart/scheduled_restart_bubble_view_browsertest.cc
@@ -20,7 +20,10 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/test/browser_test.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/events/base_event_utils.h"
+#include "ui/events/event.h"
 #include "ui/views/controls/button/md_text_button.h"
+#include "ui/views/metrics.h"
 #include "ui/views/test/button_test_api.h"
 #include "ui/views/view_utils.h"
 #include "ui/views/widget/widget.h"
@@ -127,7 +130,14 @@
       views::AsViewClass<views::MdTextButton>(dialog_delegate->GetExtraView());
   ASSERT_TRUE(idle_button);
 
-  views::test::ButtonTestApi(idle_button).NotifyDefaultMouseClick();
+  // The click timestamp must be past the dialog's input protection window,
+  // otherwise the extra button ignores it as a possibly unintended
+  // interaction.
+  views::test::ButtonTestApi(idle_button)
+      .NotifyClick(ui::MouseEvent(
+          ui::EventType::kMousePressed, gfx::Point(), gfx::Point(),
+          ui::EventTimeForNow() + 2 * views::GetDoubleClickInterval(),
+          ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON));
 
   EXPECT_TRUE(manager()->is_scheduled());
   EXPECT_EQ(manager()->mode(), ScheduledRestartMode::kOnIdle);
diff --git a/ui/views/bubble/bubble_dialog_model_host_unittest.cc b/ui/views/bubble/bubble_dialog_model_host_unittest.cc
index 6393f94..b7bcaa1 100644
--- a/ui/views/bubble/bubble_dialog_model_host_unittest.cc
+++ b/ui/views/bubble/bubble_dialog_model_host_unittest.cc
@@ -9,21 +9,30 @@
 
 #include "base/functional/callback_helpers.h"
 #include "base/memory/ptr_util.h"
+#include "base/test/bind.h"
 #include "base/test/gtest_util.h"
 #include "ui/base/interaction/element_identifier.h"
 #include "ui/base/interaction/element_tracker.h"
 #include "ui/base/models/dialog_model.h"
 #include "ui/base/mojom/dialog_button.mojom.h"
+#include "ui/events/base_event_utils.h"
+#include "ui/events/event.h"
+#include "ui/strings/grit/ui_strings.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/md_text_button.h"
+#include "ui/views/controls/link.h"
 #include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/interaction/element_tracker_views.h"
 #include "ui/views/layout/box_layout_view.h"
 #include "ui/views/metadata/view_factory.h"
+#include "ui/views/metrics.h"
+#include "ui/views/test/button_test_api.h"
 #include "ui/views/test/views_test_base.h"
+#include "ui/views/test/views_test_utils.h"
 #include "ui/views/test/widget_test.h"
 #include "ui/views/view_class_properties.h"
 #include "ui/views/view_utils.h"
+#include "ui/views/window/dialog_client_view.h"
 
 class BubbleDialogModelHostTestPassKey {
  public:
@@ -685,4 +694,194 @@
   host.reset();
 }
 
+namespace {
+
+// Shows a bubble hosting `model` and returns the widget. The host can be
+// accessed through the widget's delegate.
+Widget* ShowBubbleWithModel(std::unique_ptr<ui::DialogModel> model,
+                            Widget* anchor_widget) {
+  auto host = std::make_unique<BubbleDialogModelHost>(
+      std::move(model), anchor_widget->GetContentsView(),
+      BubbleBorder::Arrow::TOP_RIGHT);
+  Widget* const bubble_widget = BubbleDialogDelegate::CreateBubbleDeprecated(
+      std::move(host), Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET);
+  test::WidgetVisibleWaiter waiter(bubble_widget);
+  bubble_widget->Show();
+  waiter.Wait();
+  return bubble_widget;
+}
+
+BubbleDialogModelHost* GetHost(Widget* bubble_widget) {
+  return static_cast<BubbleDialogModelHost*>(
+      bubble_widget->widget_delegate()->AsDialogDelegate());
+}
+
+ui::MouseEvent MouseClickAt(base::TimeTicks time_stamp) {
+  return ui::MouseEvent(ui::EventType::kMousePressed, gfx::PointF(),
+                        gfx::PointF(), time_stamp, ui::EF_LEFT_MOUSE_BUTTON,
+                        ui::EF_LEFT_MOUSE_BUTTON);
+}
+
+}  // namespace
+
+// The extra button is not routed through DialogClientView::ButtonPressed(),
+// so BubbleDialogModelHost applies the dialog's input protection to it
+// directly. Pointer events that arrive within the protection window of the
+// dialog becoming visible, or in rapid succession, must not run the button
+// callback.
+TEST_F(BubbleDialogModelHostTest,
+       ExtraButtonIgnoresEarlyAndRepeatedPointerEvents) {
+  std::unique_ptr<Widget> anchor_widget = CreateTestWidget(
+      Widget::InitParams::CLIENT_OWNS_WIDGET, Widget::InitParams::TYPE_WINDOW);
+  anchor_widget->Show();
+
+  int press_count = 0;
+  Widget* const bubble_widget = ShowBubbleWithModel(
+      ui::DialogModel::Builder()
+          .AddOkButton(base::DoNothing())
+          .AddExtraButton(base::BindLambdaForTesting(
+                              [&](const ui::Event&) { ++press_count; }),
+                          ui::DialogModel::Button::Params().SetLabel(u"Button"))
+          .Build(),
+      anchor_widget.get());
+  BubbleDialogModelHost* const host = GetHost(bubble_widget);
+
+  auto* const extra_button = AsViewClass<Button>(host->GetExtraView());
+  ASSERT_NE(extra_button, nullptr);
+
+  // Restart the protection window so that event timestamps can be reliably
+  // compared against it.
+  host->GetDialogClientView()->TriggerInputProtection(/*force_early=*/true);
+
+  // A pointer event within the protection window is ignored.
+  test::ButtonTestApi(extra_button)
+      .NotifyClick(MouseClickAt(ui::EventTimeForNow()));
+  EXPECT_EQ(0, press_count);
+
... (file diff truncated)
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/chrome/browser/devtools/devtools_connection_dialog_browsertest.cc b/chrome/browser/devtools/devtools_connection_dialog_browsertest.cc
index 9139fa93..459cbfc 100644
--- a/chrome/browser/devtools/devtools_connection_dialog_browsertest.cc
+++ b/chrome/browser/devtools/devtools_connection_dialog_browsertest.cc
@@ -15,6 +15,7 @@
 #include "ui/events/event.h"
 #include "ui/views/bubble/bubble_dialog_model_host.h"
 #include "ui/views/controls/button/md_text_button.h"
+#include "ui/views/metrics.h"
 #include "ui/views/test/button_test_api.h"
 #include "ui/views/test/widget_test.h"
 #include "ui/views/widget/widget.h"
@@ -100,12 +101,15 @@
 
   content::WebContentsAddedObserver new_tab_observer;
 
+  // The click timestamp must be past the dialog's input protection window,
+  // otherwise the extra button ignores it as a possibly unintended
+  // interaction.
   views::test::ButtonTestApi(
       static_cast<views::MdTextButton*>(dialog_model_dialog->GetExtraView()))
-      .NotifyClick(ui::MouseEvent(ui::EventType::kMousePressed, gfx::Point(),
-                                  gfx::Point(), ui::EventTimeForNow(),
-                                  ui::EF_LEFT_MOUSE_BUTTON,
-                                  ui::EF_LEFT_MOUSE_BUTTON));
+      .NotifyClick(ui::MouseEvent(
+          ui::EventType::kMousePressed, gfx::Point(), gfx::Point(),
+          ui::EventTimeForNow() + 2 * views::GetDoubleClickInterval(),
+          ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON));
 
   EXPECT_EQ(future.Get(),
             content::DevToolsManagerDelegate::AcceptConnectionResult::kDeny);
diff --git a/chrome/browser/ssl/ask_before_http_dialog_controller_interactive_uitest.cc b/chrome/browser/ssl/ask_before_http_dialog_controller_interactive_uitest.cc
index 0536cdc..960e55b 100644
--- a/chrome/browser/ssl/ask_before_http_dialog_controller_interactive_uitest.cc
+++ b/chrome/browser/ssl/ask_before_http_dialog_controller_interactive_uitest.cc
@@ -46,7 +46,15 @@
 #include "services/metrics/public/cpp/ukm_source.h"
 #include "ui/base/metadata/metadata_impl_macros.h"
 #include "ui/base/page_transition_types.h"
+#include "ui/events/base_event_utils.h"
+#include "ui/events/event.h"
+#include "ui/views/controls/button/button.h"
 #include "ui/views/controls/styled_label.h"
+#include "ui/views/interaction/interaction_test_util_views.h"
+#include "ui/views/metrics.h"
+#include "ui/views/test/button_test_api.h"
+#include "ui/views/window/dialog_client_view.h"
+#include "ui/views/window/dialog_delegate.h"
 
 using security_interstitials::MetricsHelper;
 using security_interstitials::https_only_mode::BlockingResult;
@@ -403,6 +411,68 @@
   ExpectUKMEntry(http_url, BlockingResult::kInterstitialProceed);
 }
 
+// Mouse clicks landing on the "Continue to site" button while the dialog's
+// input protection window is active (e.g. a click arriving right as the
+// dialog appears) must be ignored, matching the protection that the standard
+// dialog buttons get from DialogClientView. A click arriving after the
+// protection window has passed should still proceed to the site.
+IN_PROC_BROWSER_TEST_P(AskBeforeHttpDialogControllerUiTest,
+                       FailedUpgrade_WarningShown_EarlyContinueClickIgnored) {
+  DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kTestTab);
+
+  GURL http_url = http_server()->GetURL("bad-https.com", "/simple.html");
+
+  auto* contents = GetBrowser()->tab_strip_model()->GetActiveWebContents();
+  content::NavigationController::LoadURLParams params(http_url);
+  params.transition_type = ui::PAGE_TRANSITION_LINK;
+  content::NavigateToURLBlockUntilNavigationsComplete(contents, params, 1);
+  EXPECT_EQ(http_url, contents->GetLastCommittedURL());
+
+  RunTestSequence(
+      InAnyContext(
+          WaitForShow(AskBeforeHttpDialogController::kContinueButtonId)),
+      InSameContext(
+          InstrumentTab(kTestTab),
+          // Simulate a mouse click on the button within the input protection
+          // window. The protection window is restarted first so that the
+          // click reliably falls inside it, regardless of how much time has
+          // passed since the dialog was shown.
+          WithView(
+              AskBeforeHttpDialogController::kContinueButtonId,
+              [](views::Button* button) {
+                button->GetWidget()
+                    ->widget_delegate()
+                    ->AsDialogDelegate()
+                    ->GetDialogClientView()
+                    ->TriggerInputProtection(/*force_early=*/true);
+                views::test::InteractionTestUtilSimulatorViews::PressButton(
+                    button, ui::test::InteractionTestUtil::InputType::kMouse);
+              }),
+          // The click must have been ignored: the dialog is still showing.
+          EnsurePresent(AskBeforeHttpDialogController::kContinueButtonId),
+          // A click arriving after the protection window has passed is
+          // accepted and continues to the site.
+          WithView(
+              AskBeforeHttpDialogController::kContinueButtonId,
+              [](views::Button* button) {
+                const ui::MouseEvent click(
+                    ui::EventType::kMousePressed, gfx::PointF(), gfx::PointF(),
+                    ui::EventTimeForNow() + 2 * views::GetDoubleClickInterval(),
+                    ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON);
+                views::test::ButtonTestApi(button).NotifyClick(click);
+              })
+              .SetMustRemainVisible(false),
+          WaitForWebContentsNavigation(kTestTab, http_url)));
+
+  // Only the delayed click should count as a decision on the warning.
+  histograms()->ExpectBucketCount("interstitial.https_first_mode.decision",
+                                  MetricsHelper::Decision::SHOW, 1);
+  histograms()->ExpectBucketCount("interstitial.https_first_mode.decision",
+                                  MetricsHelper::Decision::PROCEED, 1);
+
+  ExpectUKMEntry(http_url, BlockingResult::kInterstitialProceed);
+}
+
 // If the user triggers an Ask-before-HTTP warning for a host and then
 // clicks the "Go back" button, they should navigate back to the previous
 // page (about:blank in this case).
diff --git a/chrome/browser/ui/views/scheduled_restart/scheduled_restart_bubble_view_browsertest.cc b/chrome/browser/ui/views/scheduled_restart/scheduled_restart_bubble_view_browsertest.cc
index 5d470fa3..581d009 100644
--- a/chrome/browser/ui/views/scheduled_restart/scheduled_restart_bubble_view_browsertest.cc
+++ b/chrome/browser/ui/views/scheduled_restart/scheduled_restart_bubble_view_browsertest.cc
@@ -20,7 +20,10 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/test/browser_test.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/events/base_event_utils.h"
+#include "ui/events/event.h"
 #include "ui/views/controls/button/md_text_button.h"
+#include "ui/views/metrics.h"
 #include "ui/views/test/button_test_api.h"
 #include "ui/views/view_utils.h"
 #include "ui/views/widget/widget.h"
@@ -127,7 +130,14 @@
       views::AsViewClass<views::MdTextButton>(dialog_delegate->GetExtraView());
   ASSERT_TRUE(idle_button);
 
-  views::test::ButtonTestApi(idle_button).NotifyDefaultMouseClick();
+  // The click timestamp must be past the dialog's input protection window,
+  // otherwise the extra button ignores it as a possibly unintended
+  // interaction.
+  views::test::ButtonTestApi(idle_button)
+      .NotifyClick(ui::MouseEvent(
+          ui::EventType::kMousePressed, gfx::Point(), gfx::Point(),
+          ui::EventTimeForNow() + 2 * views::GetDoubleClickInterval(),
+          ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON));
 
   EXPECT_TRUE(manager()->is_scheduled());
   EXPECT_EQ(manager()->mode(), ScheduledRestartMode::kOnIdle);
diff --git a/ui/views/bubble/bubble_dialog_model_host_unittest.cc b/ui/views/bubble/bubble_dialog_model_host_unittest.cc
index 6393f94..b7bcaa1 100644
--- a/ui/views/bubble/bubble_dialog_model_host_unittest.cc
+++ b/ui/views/bubble/bubble_dialog_model_host_unittest.cc
@@ -9,21 +9,30 @@
 
 #include "base/functional/callback_helpers.h"
 #include "base/memory/ptr_util.h"
+#include "base/test/bind.h"
 #include "base/test/gtest_util.h"
 #include "ui/base/interaction/element_identifier.h"
 #include "ui/base/interaction/element_tracker.h"
 #include "ui/base/models/dialog_model.h"
 #include "ui/base/mojom/dialog_button.mojom.h"
+#include "ui/events/base_event_utils.h"
+#include "ui/events/event.h"
+#include "ui/strings/grit/ui_strings.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/md_text_button.h"
+#include "ui/views/controls/link.h"
 #include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/interaction/element_tracker_views.h"
 #include "ui/views/layout/box_layout_view.h"
 #include "ui/views/metadata/view_factory.h"
+#include "ui/views/metrics.h"
+#include "ui/views/test/button_test_api.h"
 #include "ui/views/test/views_test_base.h"
+#include "ui/views/test/views_test_utils.h"
 #include "ui/views/test/widget_test.h"
 #include "ui/views/view_class_properties.h"
 #include "ui/views/view_utils.h"
+#include "ui/views/window/dialog_client_view.h"
 
 class BubbleDialogModelHostTestPassKey {
  public:
@@ -685,4 +694,194 @@
   host.reset();
 }
 
+namespace {
+
+// Shows a bubble hosting `model` and returns the widget. The host can be
+// accessed through the widget's delegate.
+Widget* ShowBubbleWithModel(std::unique_ptr<ui::DialogModel> model,
+                            Widget* anchor_widget) {
+  auto host = std::make_unique<BubbleDialogModelHost>(
+      std::move(model), anchor_widget->GetContentsView(),
+      BubbleBorder::Arrow::TOP_RIGHT);
+  Widget* const bubble_widget = BubbleDialogDelegate::CreateBubbleDeprecated(
+      std::move(host), Widget::InitParams::NATIVE_WIDGET_OWNS_WIDGET);
+  test::WidgetVisibleWaiter waiter(bubble_widget);
+  bubble_widget->Show();
+  waiter.Wait();
+  return bubble_widget;
+}
+
+BubbleDialogModelHost* GetHost(Widget* bubble_widget) {
+  return static_cast<BubbleDialogModelHost*>(
+      bubble_widget->widget_delegate()->AsDialogDelegate());
+}
+
+ui::MouseEvent MouseClickAt(base::TimeTicks time_stamp) {
+  return ui::MouseEvent(ui::EventType::kMousePressed, gfx::PointF(),
+                        gfx::PointF(), time_stamp, ui::EF_LEFT_MOUSE_BUTTON,
+                        ui::EF_LEFT_MOUSE_BUTTON);
+}
+
+}  // namespace
+
+// The extra button is not routed through DialogClientView::ButtonPressed(),
+// so BubbleDialogModelHost applies the dialog's input protection to it
+// directly. Pointer events that arrive within the protection window of the
+// dialog becoming visible, or in rapid succession, must not run the button
+// callback.
+TEST_F(BubbleDialogModelHostTest,
+       ExtraButtonIgnoresEarlyAndRepeatedPointerEvents) {
+  std::unique_ptr<Widget> anchor_widget = CreateTestWidget(
+      Widget::InitParams::CLIENT_OWNS_WIDGET, Widget::InitParams::TYPE_WINDOW);
+  anchor_widget->Show();
+
+  int press_count = 0;
+  Widget* const bubble_widget = ShowBubbleWithModel(
+      ui::DialogModel::Builder()
+          .AddOkButton(base::DoNothing())
+          .AddExtraButton(base::BindLambdaForTesting(
+                              [&](const ui::Event&) { ++press_count; }),
+                          ui::DialogModel::Button::Params().SetLabel(u"Button"))
+          .Build(),
+      anchor_widget.get());
+  BubbleDialogModelHost* const host = GetHost(bubble_widget);
+
+  auto* const extra_button = AsViewClass<Button>(host->GetExtraView());
+  ASSERT_NE(extra_button, nullptr);
+
+  // Restart the protection window so that event timestamps can be reliably
+  // compared against it.
+  host->GetDialogClientView()->TriggerInputProtection(/*force_early=*/true);
+
+  // A pointer event within the protection window is ignored.
+  test::ButtonTestApi(extra_button)
+      .NotifyClick(MouseClickAt(ui::EventTimeForNow()));
+  EXPECT_EQ(0, press_count);
+
+  // A pointer event past the protection window is accepted.
+  const base::TimeTicks delayed_time =
+      ui::EventTimeForNow() + 2 * GetDoubleClickInterval();
+  test::ButtonTestApi(extra_button).NotifyClick(MouseClickAt(delayed_time));
+  EXPECT_EQ(1, press_count);
+
+  // A rapid repeat of the previous event is ignored.
+  test::ButtonTestApi(extra_button)
+      .NotifyClick(MouseClickAt(delayed_time + base::Milliseconds(1)));
+  EXPECT_EQ(1, press_count);
+
+  bubble_widget->CloseNow();
+}
+
+// Key events activate the extra button during the protection window unless
+// the model opts into input protection, matching the behavior of the ok and
+// cancel buttons.
+TEST_F(BubbleDialogModelHostTest, ExtraButtonAllowsKeyEventsByDefault) {
+  std::unique_ptr<Widget> anchor_widget = CreateTestWidget(
+      Widget::InitParams::CLIENT_OWNS_WIDGET, Widget::InitParams::TYPE_WINDOW);
+  anchor_widget->Show();
+
+  int press_count = 0;
+  Widget* const bubble_widget = ShowBubbleWithModel(
+      ui::DialogModel::Builder()
+          .AddOkButton(base::DoNothing())
+          .AddExtraButton(base::BindLambdaForTesting(
+                              [&](const ui::Event&) { ++press_count; }),
+                          ui::DialogModel::Button::Params().SetLabel(u"Button"))
+          .Build(),
+      anchor_widget.get());
+  BubbleDialogModelHost* const host = GetHost(bubble_widget);
+
+  auto* const extra_button = AsViewClass<Button>(host->GetExtraView());
+  ASSERT_NE(extra_button, nullptr);
+
+  host->GetDialogClientView()->TriggerInputProtection(/*force_early=*/true);
+
+  test::ButtonTestApi(extra_button)
+      .NotifyClick(ui::KeyEvent(ui::EventType::kKeyPressed, ui::VKEY_RETURN,
+                                ui::EF_NONE, ui::EventTimeForNow()));
+  EXPECT_EQ(1, press_count);
+
+  bubble_widget->CloseNow();
+}
... (truncated)
Loading diff…

Original Bug Report

reported by vm...@google.com

Clickjacking bypass of Ask Before HTTP dialog via extra button

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: The desktop HTTPS-First Mode ‘Ask Before HTTP’ dialog places its ‘Continue to site’ action on an extra button rather than standard dialog buttons. Because BubbleDialogModelHost binds extra button callbacks directly to the model rather than through DialogClientView::ButtonPressed, clicks on this button bypass InputEventActivationProtector entirely. An attacker could potentially use rapid clicking or clickjacking to dismiss the dialog immediately upon display and allowlist their domain for unencrypted HTTP traffic.

Affected files:

  • chrome/browser/ssl/ask_before_http_dialog_controller.cc
  • ui/views/bubble/bubble_dialog_model_host.cc

Estimated timestamp from git blame: 2025-08-06

Summary

In desktop Chromium, the HTTPS-First Mode ‘Ask Before HTTP’ modal dialog registers its dangerous action (“Continue to site”) as a ui::DialogModel extra button. While the primary OK/Cancel buttons are wired through DialogClientView::ButtonPressed (which guards against unintended interactions via views::InputEventActivationProtector), extra buttons built by BubbleDialogModelHost bind click events directly to the model’s button callback, bypassing all input protection and cooldown checks.

Technical Details

  1. Extra Button Binding (chrome/browser/ssl/ask_before_http_dialog_controller.cc:431-445): On desktop platforms (!BUILDFLAG(IS_ANDROID)), the controller adds the “Continue to site” action using builder.AddExtraButton(...):

    builder.AddExtraButton(
        base::BindRepeating(
            [](base::WeakPtr<AskBeforeHttpDialogController> controller,
               const GURL& url, const ui::Event& event) {
              if (controller) {
                controller->OnContinueButtonClicked(url);
              }
            },
            weak_ptr_factory_.GetWeakPtr(), request_url),
        ui::DialogModel::Button::Params()
            .SetLabel(l10n_util::GetStringUTF16(IDS_HTTPS_ONLY_MODE_SUBMIT_BUTTON))
            .SetStyle(ui::ButtonStyle::kDefault)
            .SetId(kContinueButtonId));
    
  2. Bypass of DialogClientView::ButtonPressed (ui/views/bubble/bubble_dialog_model_host.cc:937-950): BubbleDialogModelHost constructs the extra view as an MdTextButton and directly assigns &ui::DialogModel::Button::OnPressed as its callback:

    if (ui::DialogModel::Button* extra_button =
            model_->extra_button(DialogModelHost::GetPassKey())) {
      auto builder =
          views::Builder<MdTextButton>()
              .SetCallback(base::BindRepeating(
                  &ui::DialogModel::Button::OnPressed,
                  base::Unretained(extra_button), DialogModelHost::GetPassKey()))
      ...
      SetExtraView(std::move(builder).Build());
    }
    

    DialogModel::Button::OnPressed executes callback_.Run(event) unconditionally (ui/base/models/dialog_model.cc:73-76).

  3. Input Protection Coverage Asymmetry (ui/views/window/dialog_client_view.cc:411-422): DialogClientView::ButtonPressed is only invoked for kOk and kCancel buttons, where it performs protection checks:

    if (!delegate ||
        input_protector_->IsPossiblyUnintendedInteraction(
            event, /*allow_key_events=*/delegate
                       ->ShouldAllowKeyEventsDuringInputProtection())) {
      return;
    }
    

    Because extra buttons bypass DialogClientView::ButtonPressed and features::kEnableInputProtection is disabled by default, the extra button has no input cooldown, double-click protection, or repeated-event rejection.

Potential Attack Steps

(Note: These are potential steps based on static analysis of the call graph; our automated review agent does not have environment execution capabilities to run dynamic PoCs.)

  1. The attacker sets up attacker.example to respond over HTTP (port 80) and immediately send a TCP RST on HTTPS (port 443).
  2. The attacker hosts a page on an external domain with a decoy click game positioned at the deterministic coordinates where the modal dialog’s bottom-left extra button will appear (centered horizontally, toolbar bottom minus 3px vertically, fixed width 448px per TabModalDialogHost::GetDialogPosition).
  3. The user interacts with the decoy game in an Incognito tab (where HFM is default-on via kHttpsFirstModeIncognito) or a profile with HFM enabled.
  4. The first click initiates navigation to http://attacker.example. HttpsUpgradesInterceptor attempts an upgrade to https://attacker.example, which fails quickly (~1 RTT via TCP RST).
  5. HttpsUpgradesNavigationThrottle::WillRedirectRequest cancels the fallback redirect with a local blank page (kBlankPageHtml), and HttpsOnlyModeTabHelper::DidFinishNavigation synchronously displays the dialog.
  6. The user’s second click lands on the “Continue to site” button within the double-click window (<500ms). Because input protection is bypassed, the button click is immediately processed.
  7. AskBeforeHttpDialogController::OnContinueButtonClicked persists an HTTP_ALLOWED content setting exception via StatefulSSLHostStateDelegate::AllowHttpForHost (valid for up to 15 days in standard profiles, or the duration of the Incognito session), and reloads the host over plaintext HTTP.

Suggested Fix

Potential remediation options:

  1. Update AskBeforeHttpDialogController::OnContinueButtonClicked (or the callback lambda) to check against an InputEventActivationProtector before proceeding (similar to the approach used in FedCmAccountSelectionViewDesktop), or
  2. Integrate extra button callbacks in BubbleDialogModelHost / DialogClientView into DialogClientView::ButtonPressed or a shared InputEventActivationProtector validation check before invoking the underlying callback.

Evaluated with Chrome root at commit: c6718ebbf2c677c6f63691e28c7801c563f6b19c


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.

View on issue tracker