From 9916df4972b639b155704e52288903246b1c382a Mon Sep 17 00:00:00 2001
From: Eva Su <evasu@chromium.org>
Date: Mon, 08 Jun 2026 12:07:25 -0700
Subject: [PATCH] [Extensions] Fix clickjacking in ExtensionsRequestAccessButton

This CL addresses a potential clickjacking and UI redressing
vulnerability in the ExtensionsRequestAccessButton. A malicious
extension could previously flicker the toolbar chip programmatically
without a user gesture, inducing horizontal layout shifts that could
lead to accidental clicks granting persistent host access.

This adds input event activation protection and rate limits the
AddRequest and RemoveRequest calls in HostAccessRequestsHelper.

InputEventActivationProtector is added to the request access button
which ignores user clicks for a short duration (500ms) after the button
becomes visible or changes state, matching standard Chromium security
patterns for preventing misclicks during layout shifts.

The API rate limiting introduces a 1-second cooldown in
HostAccessRequestsHelper for both AddRequest and RemoveRequest calls
which prevents extensions from rapidly toggling the UI state to induce
layout shifts.

Fixed: 501854535
Change-Id: Ib0d95d56097aae2cbb2d806f42885400b056c6f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7876925
Reviewed-by: Andrea Orru <andreaorru@chromium.org>
Commit-Queue: Eva Su <evasu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1643382}
---

diff --git a/chrome/browser/extensions/api/permissions/permissions_api.cc b/chrome/browser/extensions/api/permissions/permissions_api.cc
index 0efbde5..d26c0822 100644
--- a/chrome/browser/extensions/api/permissions/permissions_api.cc
+++ b/chrome/browser/extensions/api/permissions/permissions_api.cc
@@ -65,6 +65,10 @@
     "any of its host permissions.";
 constexpr char kExtensionRequestCannotBeRemovedError[] =
     "Extension cannot remove a host access request that doesn't exist.";
+constexpr char kExtensionRequestRateLimitError[] =
+    "Extension cannot remove a host access request due to rate limiting.";
+constexpr char kExtensionAddRequestRateLimitError[] =
+    "Extension cannot add a host access request due to rate limiting.";
 constexpr char kAddRequestInvalidPatternError[] =
     "Extension cannot add a request with an invalid value for 'pattern'.";
 constexpr char kRemoveRequestInvalidPatternError[] =
@@ -631,8 +635,12 @@
     }
   }
 
-  permissions_manager->AddHostAccessRequest(web_contents, tab_id, *extension(),
-                                            pattern);
+  PermissionsManager::AddRequestResult result =
+      permissions_manager->AddHostAccessRequest(web_contents, tab_id,
+                                                *extension(), pattern);
+  if (result == PermissionsManager::AddRequestResult::kThrottled) {
+    return RespondNow(Error(kExtensionAddRequestRateLimitError));
+  }
   return RespondNow(NoArguments());
 }
 
@@ -693,14 +701,17 @@
   DCHECK(web_contents);
   DCHECK_NE(tab_id, -1);
 
-  bool is_removed =
+  PermissionsManager::RemoveRequestResult result =
       PermissionsManager::Get(browser_context())
           ->RemoveHostAccessRequest(tab_id, extension()->id(), pattern);
-  if (!is_removed) {
-    return RespondNow(Error(kExtensionRequestCannotBeRemovedError));
+  switch (result) {
+    case PermissionsManager::RemoveRequestResult::kSuccess:
+      return RespondNow(NoArguments());
+    case PermissionsManager::RemoveRequestResult::kNotFound:
+      return RespondNow(Error(kExtensionRequestCannotBeRemovedError));
+    case PermissionsManager::RemoveRequestResult::kThrottled:
+      return RespondNow(Error(kExtensionRequestRateLimitError));
   }
-
-  return RespondNow(NoArguments());
 }
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/permissions/permissions_api_browsertest.cc b/chrome/browser/extensions/api/permissions/permissions_api_browsertest.cc
index 13cf106..32e4a714 100644
--- a/chrome/browser/extensions/api/permissions/permissions_api_browsertest.cc
+++ b/chrome/browser/extensions/api/permissions/permissions_api_browsertest.cc
@@ -29,6 +29,7 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extension_util.h"
+#include "extensions/browser/host_access_request_helper.h"
 #include "extensions/browser/permissions/active_tab_permission_granter.h"
 #include "extensions/browser/permissions/permissions_test_util.h"
 #include "extensions/browser/permissions/permissions_updater.h"
@@ -772,6 +773,7 @@
 
   void SetUpOnMainThread() override {
     PermissionsAPIUnitTest::SetUpOnMainThread();
+    HostAccessRequestsHelper::SetCooldownForTesting(base::TimeDelta());
     host_resolver()->AddRule("*", "127.0.0.1");
     ASSERT_TRUE(embedded_test_server()->Start());
   }
@@ -1586,6 +1588,89 @@
         tab_id, extension->id()));
   }
 }
+
+// Test that adding and removing host access requests are throttled when
+// the cooldown is active.
+IN_PROC_BROWSER_TEST_F(PermissionsAPIHostAccessRequestsUnitTest,
+                       HostAccessRequestCooldown) {
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder("Extension")
+          .AddHostPermission("*://*.requested.com/*")
+          .Build();
+  AddExtensionAndWithheldPermissions(*extension);
+
+  NavigateTo("http://www.requested.com");
+  int tab_id = ExtensionTabUtil::GetTabId(
+      browser()->tab_strip_model()->GetActiveWebContents());
+
+  auto* permissions_manager = PermissionsManager::Get(profile());
+
+  // Set cooldown to 200ms.
+  HostAccessRequestsHelper::SetCooldownForTesting(base::Milliseconds(200));
+
+  // Add request. Should succeed.
+  {
+    auto function =
+        base::MakeRefCounted<PermissionsAddHostAccessRequestFunction>();
+    function->set_extension(extension.get());
+    EXPECT_TRUE(api_test_utils::RunFunction(
+        function.get(), GetFunctionParams(tab_id), profile(),
+        api_test_utils::FunctionMode::kNone));
+    EXPECT_TRUE(permissions_manager->HasActiveHostAccessRequest(
+        tab_id, extension->id()));
+  }
+
+  // Remove request immediately. Should fail due to cooldown.
+  {
+    auto function =
+        base::MakeRefCounted<PermissionsRemoveHostAccessRequestFunction>();
+    function->set_extension(extension.get());
+    std::string error = api_test_utils::RunFunctionAndReturnError(
+        function.get(), GetFunctionParams(tab_id), profile(),
+        api_test_utils::FunctionMode::kNone);
+    EXPECT_EQ(
+        "Extension cannot remove a host access request due to rate limiting.",
+        error);
+    // Request is still active.
+    EXPECT_TRUE(permissions_manager->HasActiveHostAccessRequest(
+        tab_id, extension->id()));
+  }
+
+  // Wait for cooldown to expire.
+  {
+    base::RunLoop run_loop;
+    base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask(
+        FROM_HERE, run_loop.QuitClosure(), base::Milliseconds(250));
+    run_loop.Run();
+  }
+
+  // Remove request again. Should succeed now.
+  {
+    auto function =
+        base::MakeRefCounted<PermissionsRemoveHostAccessRequestFunction>();
+    function->set_extension(extension.get());
+    EXPECT_TRUE(api_test_utils::RunFunction(
+        function.get(), GetFunctionParams(tab_id), profile(),
+        api_test_utils::FunctionMode::kNone));
+    EXPECT_FALSE(permissions_manager->HasActiveHostAccessRequest(
+        tab_id, extension->id()));
+  }
+
+  // Try to immediately re-add. Should fail due to cooldown.
+  {
+    auto function =
+        base::MakeRefCounted<PermissionsAddHostAccessRequestFunction>();
+    function->set_extension(extension.get());
+    std::string error = api_test_utils::RunFunctionAndReturnError(
+        function.get(), GetFunctionParams(tab_id), profile(),
+        api_test_utils::FunctionMode::kNone);
+    EXPECT_EQ(
+        "Extension cannot add a host access request due to rate limiting.",
+        error);
+    EXPECT_FALSE(permissions_manager->HasActiveHostAccessRequest(
+        tab_id, extension->id()));
+  }
+}
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc b/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
index 1625655..9bb6bb3 100644
--- a/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
+++ b/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
@@ -67,6 +67,7 @@

diff --git a/chrome/browser/extensions/api/permissions/permissions_api_browsertest.cc b/chrome/browser/extensions/api/permissions/permissions_api_browsertest.cc
index 13cf106..32e4a714 100644
--- a/chrome/browser/extensions/api/permissions/permissions_api_browsertest.cc
+++ b/chrome/browser/extensions/api/permissions/permissions_api_browsertest.cc
@@ -29,6 +29,7 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extension_util.h"
+#include "extensions/browser/host_access_request_helper.h"
 #include "extensions/browser/permissions/active_tab_permission_granter.h"
 #include "extensions/browser/permissions/permissions_test_util.h"
 #include "extensions/browser/permissions/permissions_updater.h"
@@ -772,6 +773,7 @@
 
   void SetUpOnMainThread() override {
     PermissionsAPIUnitTest::SetUpOnMainThread();
+    HostAccessRequestsHelper::SetCooldownForTesting(base::TimeDelta());
     host_resolver()->AddRule("*", "127.0.0.1");
     ASSERT_TRUE(embedded_test_server()->Start());
   }
@@ -1586,6 +1588,89 @@
         tab_id, extension->id()));
   }
 }
+
+// Test that adding and removing host access requests are throttled when
+// the cooldown is active.
+IN_PROC_BROWSER_TEST_F(PermissionsAPIHostAccessRequestsUnitTest,
+                       HostAccessRequestCooldown) {
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder("Extension")
+          .AddHostPermission("*://*.requested.com/*")
+          .Build();
+  AddExtensionAndWithheldPermissions(*extension);
+
+  NavigateTo("http://www.requested.com");
+  int tab_id = ExtensionTabUtil::GetTabId(
+      browser()->tab_strip_model()->GetActiveWebContents());
+
+  auto* permissions_manager = PermissionsManager::Get(profile());
+
+  // Set cooldown to 200ms.
+  HostAccessRequestsHelper::SetCooldownForTesting(base::Milliseconds(200));
+
+  // Add request. Should succeed.
+  {
+    auto function =
+        base::MakeRefCounted<PermissionsAddHostAccessRequestFunction>();
+    function->set_extension(extension.get());
+    EXPECT_TRUE(api_test_utils::RunFunction(
+        function.get(), GetFunctionParams(tab_id), profile(),
+        api_test_utils::FunctionMode::kNone));
+    EXPECT_TRUE(permissions_manager->HasActiveHostAccessRequest(
+        tab_id, extension->id()));
+  }
+
+  // Remove request immediately. Should fail due to cooldown.
+  {
+    auto function =
+        base::MakeRefCounted<PermissionsRemoveHostAccessRequestFunction>();
+    function->set_extension(extension.get());
+    std::string error = api_test_utils::RunFunctionAndReturnError(
+        function.get(), GetFunctionParams(tab_id), profile(),
+        api_test_utils::FunctionMode::kNone);
+    EXPECT_EQ(
+        "Extension cannot remove a host access request due to rate limiting.",
+        error);
+    // Request is still active.
+    EXPECT_TRUE(permissions_manager->HasActiveHostAccessRequest(
+        tab_id, extension->id()));
+  }
+
+  // Wait for cooldown to expire.
+  {
+    base::RunLoop run_loop;
+    base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask(
+        FROM_HERE, run_loop.QuitClosure(), base::Milliseconds(250));
+    run_loop.Run();
+  }
+
+  // Remove request again. Should succeed now.
+  {
+    auto function =
+        base::MakeRefCounted<PermissionsRemoveHostAccessRequestFunction>();
+    function->set_extension(extension.get());
+    EXPECT_TRUE(api_test_utils::RunFunction(
+        function.get(), GetFunctionParams(tab_id), profile(),
+        api_test_utils::FunctionMode::kNone));
+    EXPECT_FALSE(permissions_manager->HasActiveHostAccessRequest(
+        tab_id, extension->id()));
+  }
+
+  // Try to immediately re-add. Should fail due to cooldown.
+  {
+    auto function =
+        base::MakeRefCounted<PermissionsAddHostAccessRequestFunction>();
+    function->set_extension(extension.get());
+    std::string error = api_test_utils::RunFunctionAndReturnError(
+        function.get(), GetFunctionParams(tab_id), profile(),
+        api_test_utils::FunctionMode::kNone);
+    EXPECT_EQ(
+        "Extension cannot add a host access request due to rate limiting.",
+        error);
+    EXPECT_FALSE(permissions_manager->HasActiveHostAccessRequest(
+        tab_id, extension->id()));
+  }
+}
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc b/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
index 1625655..9bb6bb3 100644
--- a/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
+++ b/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
@@ -67,6 +67,7 @@
   ExtensionServiceTestBase::SetUp();
   InitializeEmptyExtensionService();
   permissions_manager_ = PermissionsManager::Get(profile());
+  HostAccessRequestsHelper::SetCooldownForTesting(base::Seconds(0));
 }
 
 void HostAccessRequestsHelperUnittest::TearDown() {
@@ -142,8 +143,9 @@
   int tab_id = ExtensionTabUtil::GetTabId(web_contents);
 
   // Try to remove a non-existent host access request. Verify nothing happens.
-  EXPECT_FALSE(permissions_manager()->RemoveHostAccessRequest(
-      tab_id, extension_A->id()));
+  EXPECT_EQ(
+      permissions_manager()->RemoveHostAccessRequest(tab_id, extension_A->id()),
+      PermissionsManager::RemoveRequestResult::kNotFound);
   EXPECT_FALSE(permissions_manager()->HasActiveHostAccessRequest(
       tab_id, extension_A->id()));
   EXPECT_FALSE(permissions_manager()->HasActiveHostAccessRequest(
@@ -169,8 +171,9 @@
 
   // Remove host access request for extension A. Verify only extension B has an
   // active request.
-  EXPECT_TRUE(permissions_manager()->RemoveHostAccessRequest(
-      tab_id, extension_A->id()));
+  EXPECT_EQ(
+      permissions_manager()->RemoveHostAccessRequest(tab_id, extension_A->id()),
+      PermissionsManager::RemoveRequestResult::kSuccess);
   EXPECT_FALSE(permissions_manager()->HasActiveHostAccessRequest(
       tab_id, extension_A->id()));
   EXPECT_TRUE(permissions_manager()->HasActiveHostAccessRequest(
@@ -229,8 +232,9 @@
   // Remove a host access request for extension without specifying a filter.
   // Verify request is no longer active, since a request without filter matches
   // all patterns.
-  EXPECT_TRUE(
-      permissions_manager()->RemoveHostAccessRequest(tab_id, extension->id()));
+  EXPECT_EQ(
+      permissions_manager()->RemoveHostAccessRequest(tab_id, extension->id()),
+      PermissionsManager::RemoveRequestResult::kSuccess);
   EXPECT_FALSE(permissions_manager()->HasActiveHostAccessRequest(
       tab_id, extension->id()));
 }
diff --git a/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc b/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc
index 5379d7a3..7c1eba4 100644
--- a/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc
+++ b/chrome/browser/ui/extensions/extensions_menu_view_model_browsertest.cc
@@ -22,6 +22,7 @@
 #include "content/public/test/test_navigation_observer.h"
 #include "extensions/browser/extension_registrar.h"
 #include "extensions/browser/extension_util.h"
+#include "extensions/browser/host_access_request_helper.h"
 #include "extensions/browser/permissions/permissions_updater.h"
 #include "extensions/browser/permissions/scripting_permissions_modifier.h"
 #include "extensions/browser/permissions/site_permissions_helper.h"
@@ -1399,6 +1400,9 @@
 // matching the action_models_ order, regardless of the order they are added.
 IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewModelBrowserTest,
                        HostAccessRequests_SortedInsertionAndRemoval) {
+  extensions::HostAccessRequestsHelper::SetCooldownForTesting(
+      base::TimeDelta());
+
   // Add 3 extensions (A, B, C) and withhold their permissions, using names that
   // ensure alphabetical order.
   auto extension_A = AddExtensionWithHostPermission("Alpha", "<all_urls>");
diff --git a/chrome/browser/ui/extensions/extensions_toolbar_view_model_browsertest.cc b/chrome/browser/ui/extensions/extensions_toolbar_view_model_browsertest.cc
index dba9a321..9e01e87 100644
--- a/chrome/browser/ui/extensions/extensions_toolbar_view_model_browsertest.cc
+++ b/chrome/browser/ui/extensions/extensions_toolbar_view_model_browsertest.cc
@@ -19,6 +19,7 @@
 #include "content/public/test/browser_test.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registrar.h"
+#include "extensions/browser/host_access_request_helper.h"
 #include "extensions/browser/permissions_manager.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/buildflags/buildflags.h"
@@ -183,6 +184,9 @@
   host_resolver()->AddRule("*", "127.0.0.1");
   ASSERT_TRUE(embedded_test_server()->Start());
 
+  extensions::HostAccessRequestsHelper::SetCooldownForTesting(
+      base::TimeDelta());
+
   toolbar_delegate_ = std::make_unique<TestExtensionsToolbarDelegate>(
       browser_window_interface());
   toolbar_model_ = std::make_unique<ExtensionsToolbarViewModel>(
diff --git a/chrome/browser/ui/views/extensions/extensions_menu_main_page_view_interactive_uitest.cc b/chrome/browser/ui/views/extensions/extensions_menu_main_page_view_interactive_uitest.cc
index e8156d89..653f51cf 100644
--- a/chrome/browser/ui/views/extensions/extensions_menu_main_page_view_interactive_uitest.cc
+++ b/chrome/browser/ui/views/extensions/extensions_menu_main_page_view_interactive_uitest.cc
@@ -37,6 +37,7 @@
 #include "content/public/test/test_navigation_observer.h"
 #include "extensions/browser/extension_host.h"
 #include "extensions/browser/extension_host_registry.h"
+#include "extensions/browser/host_access_request_helper.h"
 #include "extensions/browser/permissions/scripting_permissions_modifier.h"
 #include "extensions/browser/permissions/site_permissions_helper.h"
 #include "extensions/browser/permissions_manager.h"
@@ -279,6 +280,9 @@
 // updated.
 IN_PROC_BROWSER_TEST_F(ExtensionsMenuMainPageViewInteractiveUITest,
                        SiteAccessToggle_RunAction) {
+  extensions::HostAccessRequestsHelper::SetCooldownForTesting(
+      base::TimeDelta());
+
   ASSERT_TRUE(embedded_test_server()->Start());
 
   auto extension =
diff --git a/chrome/browser/ui/views/extensions/extensions_menu_main_page_view_unittest.cc b/chrome/browser/ui/views/extensions/extensions_menu_main_page_view_unittest.cc
index b7e85806..c33c5acb 100644
--- a/chrome/browser/ui/views/extensions/extensions_menu_main_page_view_unittest.cc
+++ b/chrome/browser/ui/views/extensions/extensions_menu_main_page_view_unittest.cc
@@ -28,6 +28,7 @@
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/extension_util.h"
+#include "extensions/browser/host_access_request_helper.h"
 #include "extensions/browser/permissions/active_tab_permission_granter.h"
 #include "extensions/browser/permissions/site_permissions_helper.h"
 #include "extensions/browser/permissions_manager.h"
@@ -1431,6 +1432,9 @@
 // extensions site access and at least 1+ extensions added a site access
 // request. Reload section is always hidden if requests section is visible.
 TEST_F(ExtensionsMenuMainPageViewUnitTest, RequestsSection) {
+  extensions::HostAccessRequestsHelper::SetCooldownForTesting(
+      base::TimeDelta());
+
   // Install two extension that requests host permissions.
   auto extension_A =
       InstallExtensionWithHostPermissions("Extension A", {"<all_urls>"});
diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_desktop_interactive_uitest.cc b/chrome/browser/ui/views/extensions/extensions_toolbar_desktop_interactive_uitest.cc
index a1aea01..75e1329 100644
--- a/chrome/browser/ui/views/extensions/extensions_toolbar_desktop_interactive_uitest.cc
+++ b/chrome/browser/ui/views/extensions/extensions_toolbar_desktop_interactive_uitest.cc
@@ -4,6 +4,7 @@
 
 #include <string>
 
+#include "base/command_line.h"
 #include "base/memory/raw_ptr.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/strings/string_util.h"
@@ -51,6 +52,7 @@
 #include "extensions/browser/extension_registrar.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
+#include "extensions/browser/host_access_request_helper.h"
 #include "extensions/browser/permissions/scripting_permissions_modifier.h"
 #include "extensions/browser/permissions_manager.h"
 #include "extensions/browser/process_manager.h"
@@ -66,6 +68,7 @@
 #include "ui/views/bubble/bubble_dialog_model_host.h"
 #include "ui/views/layout/animating_layout_manager_test_util.h"
 #include "ui/views/test/widget_test.h"
+#include "ui/views/views_switches.h"
 #include "ui/views/widget/widget.h"
 
 namespace {
@@ -108,6 +111,8 @@
 
   void SetUpOnMainThread() override {
     ExtensionsToolbarUITest::SetUpOnMainThread();
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        views::switches::kDisableInputEventActivationProtectionForTesting);
     // InProcessBrowserTest will create the browser and request the hosting
     // NativeWidget's window activate. However, the NativeWidget's window can
     // resolve this activation request asynchronously. Showing the extension
@@ -1622,6 +1627,9 @@
     InteractiveBrowserTest::SetUpOnMainThread();
     ASSERT_TRUE(embedded_test_server()->Start());
 
+    extensions::HostAccessRequestsHelper::SetCooldownForTesting(
+        base::TimeDelta());
+
     permissions_manager_ = PermissionsManager::Get(browser()->profile());
   }
diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_desktop_unittest.cc b/chrome/browser/ui/views/extensions/extensions_toolbar_desktop_unittest.cc
index 1da8824..4084aa26 100644
--- a/chrome/browser/ui/views/extensions/extensions_toolbar_desktop_unittest.cc
+++ b/chrome/browser/ui/views/extensions/extensions_toolbar_desktop_unittest.cc
@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <string>
 
... (truncated)