From 8cb852a9ba3a36a05b4a6eb83a4eef084c1db2a3 Mon Sep 17 00:00:00 2001
From: Andy Paicu <andypaicu@chromium.org>
Date: Wed, 23 Sep 2026 01:17:24 -0700
Subject: [PATCH] Add keyboard input protection to permission chooser dialogs

The permission chooser dialogs used the default behavior of the input
protector which was to not protect against keyboard events. However
there is no reason to not add this protection and it can help protect
against some (admitedly convoluted) clickjacking attacks.

Also added tests.

Fixed: 562945664
Change-Id: I972de2c8abd1aa7a61920c935c98d533fc8e57f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8424745
Commit-Queue: Andy Paicu <andypaicu@chromium.org>
Reviewed-by: Elly <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1703243}
---
diff --git a/chrome/browser/ui/views/extensions/chooser_dialog_view.cc b/chrome/browser/ui/views/extensions/chooser_dialog_view.cc
index 5e20e7d..0c0e648 100644
--- a/chrome/browser/ui/views/extensions/chooser_dialog_view.cc
+++ b/chrome/browser/ui/views/extensions/chooser_dialog_view.cc
@@ -83,6 +83,10 @@
   return GetCancelButton();
 }
 
+bool ChooserDialogView::ShouldAllowKeyEventsDuringInputProtection() const {
+  return false;
+}
+
 void ChooserDialogView::OnSelectionChanged() {
   DialogModelChanged();
 }
diff --git a/chrome/browser/ui/views/extensions/chooser_dialog_view.h b/chrome/browser/ui/views/extensions/chooser_dialog_view.h
index d805672d..0299704 100644
--- a/chrome/browser/ui/views/extensions/chooser_dialog_view.h
+++ b/chrome/browser/ui/views/extensions/chooser_dialog_view.h
@@ -35,6 +35,7 @@
   // views::DialogDelegate:
   bool IsDialogButtonEnabled(ui::mojom::DialogButton button) const override;
   views::View* GetInitiallyFocusedView() override;
+  bool ShouldAllowKeyEventsDuringInputProtection() const override;
 
   // views::TableViewObserver:
   void OnSelectionChanged() override;
diff --git a/chrome/browser/ui/views/permissions/chooser_bubble_ui.cc b/chrome/browser/ui/views/permissions/chooser_bubble_ui.cc
index 6e07dd4..6f672ac 100644
--- a/chrome/browser/ui/views/permissions/chooser_bubble_ui.cc
+++ b/chrome/browser/ui/views/permissions/chooser_bubble_ui.cc
@@ -87,8 +87,9 @@
   std::u16string GetWindowTitle() const override;
 
   // views::DialogDelegate:
-  bool IsDialogButtonEnabled(ui::mojom::DialogButton button) const override;
   views::View* GetInitiallyFocusedView() override;
+  bool ShouldAllowKeyEventsDuringInputProtection() const override;
+  bool IsDialogButtonEnabled(ui::mojom::DialogButton button) const override;
 
   // views::TableViewObserver:
   void OnSelectionChanged() override;
@@ -173,6 +174,11 @@
   return GetCancelButton();
 }
 
+bool ChooserBubbleUiViewDelegate::ShouldAllowKeyEventsDuringInputProtection()
+    const {
+  return false;
+}
+
 bool ChooserBubbleUiViewDelegate::IsDialogButtonEnabled(
     ui::mojom::DialogButton button) const {
   return device_chooser_content_view_->IsDialogButtonEnabled(button);
diff --git a/chrome/browser/ui/views/device_chooser_browsertest.cc b/chrome/browser/ui/views/device_chooser_browsertest.cc
index 6c3af3cb..7fe67c3 100644
--- a/chrome/browser/ui/views/device_chooser_browsertest.cc
+++ b/chrome/browser/ui/views/device_chooser_browsertest.cc
@@ -35,6 +35,12 @@
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "ui/base/page_transition_types.h"
+#include "ui/events/base_event_utils.h"
+#include "ui/views/controls/button/md_text_button.h"
+#include "ui/views/controls/table/table_view.h"
+#include "ui/views/test/button_test_api.h"
+#include "ui/views/view_utils.h"
+#include "ui/views/widget/any_widget_observer.h"
 
 namespace {
 
@@ -314,3 +320,65 @@
                                "document.documentElement.requestFullscreen()"));
   EXPECT_FALSE(fullscreen_controller->IsTabFullscreen());
 }
+
+IN_PROC_BROWSER_TEST_F(DeviceChooserBubbleTest,
+                       KeyjackingProtectionSafetyWindow) {
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), GetURL("example.com")));
+
+  views::NamedWidgetShownWaiter waiter(views::test::AnyWidgetTestPasskey{},
+                                       "ChooserBubbleUiViewDelegate");
+  ShowChooserBubble(browser(), std::make_unique<FakeUsbChooserController>(1));
+  views::Widget* bubble_widget = waiter.WaitIfNeededAndGet();
+  ASSERT_NE(bubble_widget, nullptr);
+
+  views::DialogDelegate* delegate =
+      bubble_widget->widget_delegate()->AsDialogDelegate();
+  ASSERT_NE(delegate, nullptr);
+  EXPECT_FALSE(delegate->ShouldAllowKeyEventsDuringInputProtection());
+
+  // Select the first device in the table so the OK ("Connect") button is
+  // enabled (`DialogDelegate::AcceptDialog()` DCHECKs `IsDialogButtonEnabled`).
+  auto find_table_view = [](this auto&& self,
+                            views::View* root) -> views::TableView* {
+    if (auto* table = views::AsViewClass<views::TableView>(root)) {
+      return table;
+    }
+    for (views::View* child : root->children()) {
+      if (auto* table = self(child)) {
+        return table;
+      }
+    }
+    return nullptr;
+  };
+  views::TableView* table_view = find_table_view(delegate->GetContentsView());
+  ASSERT_NE(table_view, nullptr);
+  table_view->Select(0);
+  ASSERT_TRUE(delegate->IsDialogButtonEnabled(ui::mojom::DialogButton::kOk));
+
+  views::MdTextButton* ok_button = delegate->GetOkButton();
+  ASSERT_NE(ok_button, nullptr);
+
+  // 1. Press Enter immediately (within the 500ms input protection window).
+  views::test::ButtonTestApi(ok_button).NotifyClick(
+      ui::KeyEvent(ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_NONE,
+                   ui::EventTimeForNow()));
+
+  // Verify that the bubble did not close (the key event was ignored).
+  EXPECT_FALSE(bubble_widget->IsClosed());
+
+  // 2. A repeated Enter key event (holding Enter down) should also be ignored
+  // even after the 500ms safety window.
+  views::test::ButtonTestApi(ok_button).NotifyClick(ui::KeyEvent(
+      ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_IS_REPEAT,
+      ui::EventTimeForNow() + base::Milliseconds(600)));
+  EXPECT_FALSE(bubble_widget->IsClosed());
+
+  // 3. A fresh Enter key press after the 500ms safety window (offset by 600ms)
+  // should succeed.
+  views::test::ButtonTestApi(ok_button).NotifyClick(
+      ui::KeyEvent(ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_NONE,
+                   ui::EventTimeForNow() + base::Milliseconds(600)));
+
+  // Verify that the bubble accepted and closed.
+  EXPECT_TRUE(bubble_widget->IsClosed());
+}
diff --git a/chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc b/chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc
index 7f023cf..8387393 100644
--- a/chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc
+++ b/chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc
@@ -17,6 +17,7 @@
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/md_text_button.h"
 #include "ui/views/controls/table/table_view.h"
+#include "ui/views/test/button_test_api.h"
 #include "ui/views/widget/widget.h"
 
 using permissions::FakeBluetoothChooserController;
@@ -88,10 +89,10 @@
   raw_ptr<ChooserDialogView, DanglingUntriaged> dialog_ = nullptr;
   raw_ptr<FakeBluetoothChooserController, DanglingUntriaged> controller_ =
       nullptr;
+  raw_ptr<views::Widget, DanglingUntriaged> widget_ = nullptr;
 
  private:
   std::unique_ptr<views::Widget> parent_widget_;
-  raw_ptr<views::Widget, DanglingUntriaged> widget_ = nullptr;
 };
 
 TEST_F(ChooserDialogViewTest, ButtonState) {
@@ -150,6 +151,40 @@
   dialog_->Accept();
 }
 
+TEST_F(ChooserDialogViewTest, KeyEventsBlockedDuringInputProtection) {
+  EXPECT_FALSE(dialog_->ShouldAllowKeyEventsDuringInputProtection());
+
+  AddDevice();
+  table_view()->Select(0);
+  ASSERT_TRUE(dialog_->IsDialogButtonEnabled(ui::mojom::DialogButton::kOk));
+  views::MdTextButton* ok_button = dialog_->GetOkButton();
+  ASSERT_NE(ok_button, nullptr);
+
+  // Pressing Enter immediately (within the 500ms input protection window)
+  // should be ignored.
+  EXPECT_CALL(*controller_, Select(testing::_)).Times(0);
+  views::test::ButtonTestApi(ok_button).NotifyClick(
+      ui::KeyEvent(ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_NONE,
+                   ui::EventTimeForNow()));
+  EXPECT_FALSE(widget_->IsClosed());
+
+  // A repeated Enter key event (holding Enter down from before the dialog
+  // opened) should still be ignored even after the 500ms safety window.
+  views::test::ButtonTestApi(ok_button).NotifyClick(ui::KeyEvent(
+      ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_IS_REPEAT,
+      ui::EventTimeForNow() + base::Milliseconds(600)));
+  EXPECT_FALSE(widget_->IsClosed());
+  testing::Mock::VerifyAndClearExpectations(controller_);
+
+  // A fresh Enter key press after the safety window (600ms later) should
+  // succeed.
+  EXPECT_CALL(*controller_, Select(testing::ElementsAre(0u))).Times(1);
+  views::test::ButtonTestApi(ok_button).NotifyClick(
+      ui::KeyEvent(ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_NONE,
+                   ui::EventTimeForNow() + base::Milliseconds(600)));
+  EXPECT_TRUE(widget_->IsClosed());
+}
+
 TEST_F(ChooserDialogViewTest, Cancel) {
   EXPECT_CALL(*controller_, Cancel()).Times(1);
   dialog_->Cancel();

diff --git a/chrome/browser/ui/views/device_chooser_browsertest.cc b/chrome/browser/ui/views/device_chooser_browsertest.cc
index 6c3af3cb..7fe67c3 100644
--- a/chrome/browser/ui/views/device_chooser_browsertest.cc
+++ b/chrome/browser/ui/views/device_chooser_browsertest.cc
@@ -35,6 +35,12 @@
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "ui/base/page_transition_types.h"
+#include "ui/events/base_event_utils.h"
+#include "ui/views/controls/button/md_text_button.h"
+#include "ui/views/controls/table/table_view.h"
+#include "ui/views/test/button_test_api.h"
+#include "ui/views/view_utils.h"
+#include "ui/views/widget/any_widget_observer.h"
 
 namespace {
 
@@ -314,3 +320,65 @@
                                "document.documentElement.requestFullscreen()"));
   EXPECT_FALSE(fullscreen_controller->IsTabFullscreen());
 }
+
+IN_PROC_BROWSER_TEST_F(DeviceChooserBubbleTest,
+                       KeyjackingProtectionSafetyWindow) {
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), GetURL("example.com")));
+
+  views::NamedWidgetShownWaiter waiter(views::test::AnyWidgetTestPasskey{},
+                                       "ChooserBubbleUiViewDelegate");
+  ShowChooserBubble(browser(), std::make_unique<FakeUsbChooserController>(1));
+  views::Widget* bubble_widget = waiter.WaitIfNeededAndGet();
+  ASSERT_NE(bubble_widget, nullptr);
+
+  views::DialogDelegate* delegate =
+      bubble_widget->widget_delegate()->AsDialogDelegate();
+  ASSERT_NE(delegate, nullptr);
+  EXPECT_FALSE(delegate->ShouldAllowKeyEventsDuringInputProtection());
+
+  // Select the first device in the table so the OK ("Connect") button is
+  // enabled (`DialogDelegate::AcceptDialog()` DCHECKs `IsDialogButtonEnabled`).
+  auto find_table_view = [](this auto&& self,
+                            views::View* root) -> views::TableView* {
+    if (auto* table = views::AsViewClass<views::TableView>(root)) {
+      return table;
+    }
+    for (views::View* child : root->children()) {
+      if (auto* table = self(child)) {
+        return table;
+      }
+    }
+    return nullptr;
+  };
+  views::TableView* table_view = find_table_view(delegate->GetContentsView());
+  ASSERT_NE(table_view, nullptr);
+  table_view->Select(0);
+  ASSERT_TRUE(delegate->IsDialogButtonEnabled(ui::mojom::DialogButton::kOk));
+
+  views::MdTextButton* ok_button = delegate->GetOkButton();
+  ASSERT_NE(ok_button, nullptr);
+
+  // 1. Press Enter immediately (within the 500ms input protection window).
+  views::test::ButtonTestApi(ok_button).NotifyClick(
+      ui::KeyEvent(ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_NONE,
+                   ui::EventTimeForNow()));
+
+  // Verify that the bubble did not close (the key event was ignored).
+  EXPECT_FALSE(bubble_widget->IsClosed());
+
+  // 2. A repeated Enter key event (holding Enter down) should also be ignored
+  // even after the 500ms safety window.
+  views::test::ButtonTestApi(ok_button).NotifyClick(ui::KeyEvent(
+      ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_IS_REPEAT,
+      ui::EventTimeForNow() + base::Milliseconds(600)));
+  EXPECT_FALSE(bubble_widget->IsClosed());
+
+  // 3. A fresh Enter key press after the 500ms safety window (offset by 600ms)
+  // should succeed.
+  views::test::ButtonTestApi(ok_button).NotifyClick(
+      ui::KeyEvent(ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_NONE,
+                   ui::EventTimeForNow() + base::Milliseconds(600)));
+
+  // Verify that the bubble accepted and closed.
+  EXPECT_TRUE(bubble_widget->IsClosed());
+}
diff --git a/chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc b/chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc
index 7f023cf..8387393 100644
--- a/chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc
+++ b/chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc
@@ -17,6 +17,7 @@
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/md_text_button.h"
 #include "ui/views/controls/table/table_view.h"
+#include "ui/views/test/button_test_api.h"
 #include "ui/views/widget/widget.h"
 
 using permissions::FakeBluetoothChooserController;
@@ -88,10 +89,10 @@
   raw_ptr<ChooserDialogView, DanglingUntriaged> dialog_ = nullptr;
   raw_ptr<FakeBluetoothChooserController, DanglingUntriaged> controller_ =
       nullptr;
+  raw_ptr<views::Widget, DanglingUntriaged> widget_ = nullptr;
 
  private:
   std::unique_ptr<views::Widget> parent_widget_;
-  raw_ptr<views::Widget, DanglingUntriaged> widget_ = nullptr;
 };
 
 TEST_F(ChooserDialogViewTest, ButtonState) {
@@ -150,6 +151,40 @@
   dialog_->Accept();
 }
 
+TEST_F(ChooserDialogViewTest, KeyEventsBlockedDuringInputProtection) {
+  EXPECT_FALSE(dialog_->ShouldAllowKeyEventsDuringInputProtection());
+
+  AddDevice();
+  table_view()->Select(0);
+  ASSERT_TRUE(dialog_->IsDialogButtonEnabled(ui::mojom::DialogButton::kOk));
+  views::MdTextButton* ok_button = dialog_->GetOkButton();
+  ASSERT_NE(ok_button, nullptr);
+
+  // Pressing Enter immediately (within the 500ms input protection window)
+  // should be ignored.
+  EXPECT_CALL(*controller_, Select(testing::_)).Times(0);
+  views::test::ButtonTestApi(ok_button).NotifyClick(
+      ui::KeyEvent(ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_NONE,
+                   ui::EventTimeForNow()));
+  EXPECT_FALSE(widget_->IsClosed());
+
+  // A repeated Enter key event (holding Enter down from before the dialog
+  // opened) should still be ignored even after the 500ms safety window.
+  views::test::ButtonTestApi(ok_button).NotifyClick(ui::KeyEvent(
+      ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_IS_REPEAT,
+      ui::EventTimeForNow() + base::Milliseconds(600)));
+  EXPECT_FALSE(widget_->IsClosed());
+  testing::Mock::VerifyAndClearExpectations(controller_);
+
+  // A fresh Enter key press after the safety window (600ms later) should
+  // succeed.
+  EXPECT_CALL(*controller_, Select(testing::ElementsAre(0u))).Times(1);
+  views::test::ButtonTestApi(ok_button).NotifyClick(
+      ui::KeyEvent(ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_NONE,
+                   ui::EventTimeForNow() + base::Milliseconds(600)));
+  EXPECT_TRUE(widget_->IsClosed());
+}
+
 TEST_F(ChooserDialogViewTest, Cancel) {
   EXPECT_CALL(*controller_, Cancel()).Times(1);
   dialog_->Cancel();