PatchGap
Chrome · Extensions

CVE-2026-13957

Logic Error in Extensions
Medium chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactIncorrect security UI in Extensions
DescriptionIncorrect security UI in Extensions
ComponentExtensions
Bug ClassLogic Error
Tracker513553557
Fix commite6f8ab61283e (chromium/src) +396/-108
CISA KEVNot listed
CreditedGoogle
Disclosed2026-06-30

Files Changed

  • chrome/browser/extensions/extension_context_menu_model.cc
  • chrome/browser/extensions/extension_context_menu_model_browsertest.cc
  • chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
  • chrome/browser/extensions/permissions/site_permissions_helper_browsertest.cc

Patch

view commit
From e6f8ab61283e2eca8ee75188cfb0421215215292 Mon Sep 17 00:00:00 2001
From: Eva Su <evasu@chromium.org>
Date: Thu, 28 May 2026 16:58:51 -0700
Subject: [PATCH] [Extensions] Fix TOCTOU vulnerability in site access toggle

This CL fixes a Time-of-Check Time-of-Use (TOCTOU) vulnerability in the
extensions menu site access toggle, and implements the suggested fix by
binding the site-access action to the origin that was active when the
user initiated the click.

Previously, the target origin for permission changes was resolved from
the active WebContents at the moment of the click. Because the menu
remains open during page navigations, an attacker could initiate a
navigation to a sensitive site right before the click was processed,
leading to the extension being granted access to the new, sensitive
origin instead of the original one.

We also add checks for IsRestrictedUrl to prevent potential race
conditions where the UI to allow changing permission settings for a site
even if the extension is fundamentally barred from running there. These
checks ensure that permission changes are only considered for valid,
non-restricted origins and that the UI state correctly reflects that
permissions cannot be granted or altered on these sensitive sites.

For full consistency, we should also update AllowHostAccessRequest() to
use the target_origin, however, this is lower priority since it’s
typically triggered by a more immediate UI action, but I’ll handle this
separately in a follow-up CL (crrev.com/c/7865583) since this one is
already getting quite large.

Fixed: 513553557
Change-Id: I7bb8dbe11665cef4a1076fd748d256683a9035f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7853880
Commit-Queue: Eva Su <evasu@chromium.org>
Reviewed-by: Tim <tjudkins@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1638068}
---

diff --git a/chrome/browser/extensions/extension_context_menu_model.cc b/chrome/browser/extensions/extension_context_menu_model.cc
index dec7b8f..1d161ede 100644
--- a/chrome/browser/extensions/extension_context_menu_model.cc
+++ b/chrome/browser/extensions/extension_context_menu_model.cc
@@ -401,6 +401,10 @@
       command_id == PAGE_ACCESS_RUN_ON_SITE ||
       command_id == PAGE_ACCESS_RUN_ON_ALL_SITES) {
     auto* permissions = PermissionsManager::Get(profile_);
+    if (extension->permissions_data()->IsRestrictedUrl(origin_.GetURL(),
+                                                       nullptr)) {
+      return false;
+    }
     PermissionsManager::UserSiteAccess current_access =
         permissions->GetUserSiteAccess(*extension, origin_.GetURL());
     return current_access == CommandIdToSiteAccess(command_id);
@@ -634,7 +638,7 @@
 
       SitePermissionsHelper permissions(profile_);
       permissions.UpdateSiteAccess(*extension, web_contents,
-                                   CommandIdToSiteAccess(command_id));
+                                   CommandIdToSiteAccess(command_id), origin_);
       break;
     }
     case PAGE_ACCESS_PERMISSIONS_PAGE:
diff --git a/chrome/browser/extensions/extension_context_menu_model_browsertest.cc b/chrome/browser/extensions/extension_context_menu_model_browsertest.cc
index 3143df4..236db25 100644
--- a/chrome/browser/extensions/extension_context_menu_model_browsertest.cc
+++ b/chrome/browser/extensions/extension_context_menu_model_browsertest.cc
@@ -1560,8 +1560,9 @@
   // Update kOriginalUrl to have "on site" site access. This will make all other
   // non-restricted urls to have "on click" site access.
   SitePermissionsHelper permissions(profile());
-  permissions.UpdateSiteAccess(*extension, web_contents,
-                               PermissionsManager::UserSiteAccess::kOnSite);
+  permissions.UpdateSiteAccess(
+      *extension, web_contents, PermissionsManager::UserSiteAccess::kOnSite,
+      web_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin());
 
   PermissionsManager* permissions_manager = PermissionsManager::Get(profile());
   EXPECT_EQ(permissions_manager->GetUserSiteAccess(*extension, kOriginalUrl),
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 12bbb76..1625655 100644
--- a/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
+++ b/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
@@ -360,8 +360,9 @@
   // Grant "always on this host" access to the extension.
   PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
   SitePermissionsHelper permissions(profile());
-  permissions.UpdateSiteAccess(*extension, web_contents,
-                               PermissionsManager::UserSiteAccess::kOnSite);
+  permissions.UpdateSiteAccess(
+      *extension, web_contents, PermissionsManager::UserSiteAccess::kOnSite,
+      web_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   waiter.WaitForExtensionPermissionsUpdate();
 
   // Request should be removed since extension has granted host access.
diff --git a/chrome/browser/extensions/permissions/site_permissions_helper_browsertest.cc b/chrome/browser/extensions/permissions/site_permissions_helper_browsertest.cc
index 48bf511..d7ec7c5 100644
--- a/chrome/browser/extensions/permissions/site_permissions_helper_browsertest.cc
+++ b/chrome/browser/extensions/permissions/site_permissions_helper_browsertest.cc
@@ -26,6 +26,7 @@
 #include "extensions/test/extension_test_message_listener.h"
 #include "net/dns/mock_host_resolver.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/origin.h"
 
 static_assert(BUILDFLAG(ENABLE_EXTENSIONS_CORE));
 
@@ -162,8 +163,9 @@
       ReloadPageDialogController::AcceptDialogForTesting(true);
 
   // on all sites -> on site
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
   // We assume that there is only ever one action that wants to run for the test
@@ -173,8 +175,9 @@
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on site -> on-click (refresh needed due to revoking permissions)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnClick);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnClick,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnClick);
   ASSERT_TRUE(WaitForReloadToFinish());
@@ -183,8 +186,9 @@
 
   // on click -> on site (refresh needed due to script wanting to load at
   // start)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
   ASSERT_TRUE(WaitForReloadToFinish());
@@ -192,16 +196,18 @@
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on site -> on all sites
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnAllSites);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnAllSites,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnAllSites);
   ASSERT_TRUE(ContentScriptInjected());
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on all sites -> on-click (refresh needed due to revoking permissions)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnClick);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnClick,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnClick);
   EXPECT_TRUE(WaitForReloadToFinish());
@@ -236,8 +242,9 @@
       ReloadPageDialogController::AcceptDialogForTesting(true);
 
   // on all sites -> on site
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
   // We assume that there is only ever one action that wants to run for the test
@@ -247,8 +254,9 @@
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on site -> on-click (refresh needed due to revoking permissions)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnClick);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnClick,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnClick);
   EXPECT_TRUE(ContentScriptInjected() && !ExtensionWantsToRun());
@@ -258,8 +266,9 @@
 
   // on click -> on site (refresh needed due to script wanting to load at
   // start)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/chrome/browser/extensions/extension_context_menu_model_browsertest.cc b/chrome/browser/extensions/extension_context_menu_model_browsertest.cc
index 3143df4..236db25 100644
--- a/chrome/browser/extensions/extension_context_menu_model_browsertest.cc
+++ b/chrome/browser/extensions/extension_context_menu_model_browsertest.cc
@@ -1560,8 +1560,9 @@
   // Update kOriginalUrl to have "on site" site access. This will make all other
   // non-restricted urls to have "on click" site access.
   SitePermissionsHelper permissions(profile());
-  permissions.UpdateSiteAccess(*extension, web_contents,
-                               PermissionsManager::UserSiteAccess::kOnSite);
+  permissions.UpdateSiteAccess(
+      *extension, web_contents, PermissionsManager::UserSiteAccess::kOnSite,
+      web_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin());
 
   PermissionsManager* permissions_manager = PermissionsManager::Get(profile());
   EXPECT_EQ(permissions_manager->GetUserSiteAccess(*extension, kOriginalUrl),
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 12bbb76..1625655 100644
--- a/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
+++ b/chrome/browser/extensions/permissions/host_access_requests_helper_unittest.cc
@@ -360,8 +360,9 @@
   // Grant "always on this host" access to the extension.
   PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
   SitePermissionsHelper permissions(profile());
-  permissions.UpdateSiteAccess(*extension, web_contents,
-                               PermissionsManager::UserSiteAccess::kOnSite);
+  permissions.UpdateSiteAccess(
+      *extension, web_contents, PermissionsManager::UserSiteAccess::kOnSite,
+      web_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   waiter.WaitForExtensionPermissionsUpdate();
 
   // Request should be removed since extension has granted host access.
diff --git a/chrome/browser/extensions/permissions/site_permissions_helper_browsertest.cc b/chrome/browser/extensions/permissions/site_permissions_helper_browsertest.cc
index 48bf511..d7ec7c5 100644
--- a/chrome/browser/extensions/permissions/site_permissions_helper_browsertest.cc
+++ b/chrome/browser/extensions/permissions/site_permissions_helper_browsertest.cc
@@ -26,6 +26,7 @@
 #include "extensions/test/extension_test_message_listener.h"
 #include "net/dns/mock_host_resolver.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/origin.h"
 
 static_assert(BUILDFLAG(ENABLE_EXTENSIONS_CORE));
 
@@ -162,8 +163,9 @@
       ReloadPageDialogController::AcceptDialogForTesting(true);
 
   // on all sites -> on site
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
   // We assume that there is only ever one action that wants to run for the test
@@ -173,8 +175,9 @@
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on site -> on-click (refresh needed due to revoking permissions)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnClick);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnClick,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnClick);
   ASSERT_TRUE(WaitForReloadToFinish());
@@ -183,8 +186,9 @@
 
   // on click -> on site (refresh needed due to script wanting to load at
   // start)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
   ASSERT_TRUE(WaitForReloadToFinish());
@@ -192,16 +196,18 @@
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on site -> on all sites
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnAllSites);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnAllSites,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnAllSites);
   ASSERT_TRUE(ContentScriptInjected());
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on all sites -> on-click (refresh needed due to revoking permissions)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnClick);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnClick,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnClick);
   EXPECT_TRUE(WaitForReloadToFinish());
@@ -236,8 +242,9 @@
       ReloadPageDialogController::AcceptDialogForTesting(true);
 
   // on all sites -> on site
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
   // We assume that there is only ever one action that wants to run for the test
@@ -247,8 +254,9 @@
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on site -> on-click (refresh needed due to revoking permissions)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnClick);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnClick,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnClick);
   EXPECT_TRUE(ContentScriptInjected() && !ExtensionWantsToRun());
@@ -258,8 +266,9 @@
 
   // on click -> on site (refresh needed due to script wanting to load at
   // start)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
   EXPECT_TRUE(!ContentScriptInjected() && ExtensionWantsToRun());
@@ -268,16 +277,18 @@
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on site -> on all sites
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnAllSites);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnAllSites,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnAllSites);
   ASSERT_TRUE(ContentScriptInjected());
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on all sites -> on-click (refresh needed due to revoking permissions)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnClick);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnClick,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnClick);
   EXPECT_TRUE(ContentScriptInjected() && !ExtensionWantsToRun());
@@ -351,7 +362,10 @@
     // on all sites -> on click (revokes access)
     BlockedActionWaiter blocked_action_waiter(active_action_runner());
     permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                          UserSiteAccess::kOnClick);
+                                          UserSiteAccess::kOnClick,
+                                          GetActiveWebContents()
+                                              ->GetPrimaryMainFrame()
+                                              ->GetLastCommittedOrigin());
     ASSERT_EQ(
         permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
         UserSiteAccess::kOnClick);
@@ -366,8 +380,9 @@
 
   ExtensionTestMessageListener listener("injection succeeded");
   // on click -> on site (grants site access and active tab permission)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   ASSERT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
   ASSERT_EQ(permissions_helper_->GetSiteInteraction(*extension_,
@@ -382,7 +397,10 @@
     // permissions)
     BlockedActionWaiter blocked_action_waiter(active_action_runner());
     permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                          UserSiteAccess::kOnClick);
+                                          UserSiteAccess::kOnClick,
+                                          GetActiveWebContents()
+                                              ->GetPrimaryMainFrame()
+                                              ->GetLastCommittedOrigin());
     ASSERT_EQ(
         permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
         UserSiteAccess::kOnClick);
@@ -420,7 +438,10 @@
     // on all sites -> on click (revokes access)
     BlockedActionWaiter blocked_action_waiter(active_action_runner());
     permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                          UserSiteAccess::kOnClick);
+                                          UserSiteAccess::kOnClick,
+                                          GetActiveWebContents()
+                                              ->GetPrimaryMainFrame()
+                                              ->GetLastCommittedOrigin());
     ASSERT_EQ(
         permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
         UserSiteAccess::kOnClick);
@@ -442,8 +463,9 @@
 
   // on click -> on site (grants site access and redundantly active tab
   // permission)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   ASSERT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
   ASSERT_EQ(permissions_helper_->GetSiteInteraction(*extension_,
@@ -457,7 +479,10 @@
     // permissions)
     BlockedActionWaiter blocked_action_waiter(active_action_runner());
     permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                          UserSiteAccess::kOnClick);
+                                          UserSiteAccess::kOnClick,
+                                          GetActiveWebContents()
+                                              ->GetPrimaryMainFrame()
+                                              ->GetLastCommittedOrigin());
     ASSERT_EQ(
         permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
         UserSiteAccess::kOnClick);
@@ -596,8 +621,9 @@
       ReloadPageDialogController::AcceptDialogForTesting(true);
 
   // on all sites -> on click (revokes access)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnClick);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnClick,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   ASSERT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnClick);
   ASSERT_EQ(permissions_helper_->GetSiteInteraction(*extension_,
@@ -609,8 +635,9 @@
 
   ExtensionTestMessageListener listener("injection succeeded");
   // on click -> on site (grants site access and active tab permission)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnSite);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnSite,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   ASSERT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnSite);
   ASSERT_EQ(permissions_helper_->GetSiteInteraction(*extension_,
@@ -621,8 +648,9 @@
   ASSERT_FALSE(ExtensionWantsToRun());
 
   // on site -> on-click (should remove site access and active tab permissions)
-  permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                        UserSiteAccess::kOnClick);
+  permissions_helper_->UpdateSiteAccess(
+      *extension_, GetActiveWebContents(), UserSiteAccess::kOnClick,
+      GetActiveWebContents()->GetPrimaryMainFrame()->GetLastCommittedOrigin());
   EXPECT_EQ(permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
             UserSiteAccess::kOnClick);
   EXPECT_EQ(permissions_helper_->GetSiteInteraction(*extension_,
@@ -696,7 +724,10 @@
     // on all sites -> on site.
     ExtensionTestMessageListener listener("success");
     permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                          UserSiteAccess::kOnSite);
+                                          UserSiteAccess::kOnSite,
+                                          GetActiveWebContents()
+                                              ->GetPrimaryMainFrame()
+                                              ->GetLastCommittedOrigin());
     EXPECT_EQ(
         permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
         UserSiteAccess::kOnSite);
@@ -711,7 +742,10 @@
     // on site -> on-click (refresh needed due to revoking permissions).
     BlockedActionWaiter blocked_action_waiter(active_action_runner());
     permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                          UserSiteAccess::kOnClick);
+                                          UserSiteAccess::kOnClick,
+                                          GetActiveWebContents()
+                                              ->GetPrimaryMainFrame()
+                                              ->GetLastCommittedOrigin());
     EXPECT_EQ(
         permissions_manager_->GetUserSiteAccess(*extension_, original_url_),
         UserSiteAccess::kOnClick);
@@ -725,7 +759,10 @@
     // on click -> on site
     ExtensionTestMessageListener listener("success");
     permissions_helper_->UpdateSiteAccess(*extension_, GetActiveWebContents(),
-                                          UserSiteAccess::kOnSite);
+                                          UserSiteAccess::kOnSite,
... (truncated)
Loading diff…

Original Bug Report

reported by vm...@google.com

Potential persistent site access grant TOCTOU in Extensions menu toggle

Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.

Overview: A Time-of-Check Time-of-Use (TOCTOU) vulnerability in the Extensions menu allows an attacker to potentially trick a user into granting persistent site permissions to a malicious extension for a sensitive origin. This occurs because the target origin is resolved from the active WebContents at the time of the click rather than being bound to the origin displayed when the menu was rendered.

Affected files:

  • chrome/browser/ui/extensions/extensions_menu_view_model.cc
  • extensions/browser/permissions/site_permissions_helper.cc
  • chrome/browser/ui/views/extensions/extensions_menu_delegate_desktop.cc
  • chrome/browser/ui/android/extensions/extensions_menu_delegate_android.cc

Estimated timestamp from git blame: 2023-05-14

Summary

A potential Time-of-Check Time-of-Use (TOCTOU) vulnerability exists in the Extensions menu’s site-access management logic (puzzle-piece icon). When a user interacts with the site-access toggle for an extension, the origin for which access is granted is determined by querying the active WebContents at the moment of the click. Because the Extensions menu remains open during page navigations and updates in-place, an attacker can potentially race a navigation to a sensitive origin to intercept the permission grant.

Vulnerability Details

In ExtensionsMenuViewModel::GrantSiteAccess (and similarly in RevokeSiteAccess), the target URL is retrieved from the active WebContents using GetLastCommittedURL():

// chrome/browser/ui/extensions/extensions_menu_view_model.cc
void ExtensionsMenuViewModel::GrantSiteAccess(const extensions::ExtensionId& extension_id) {
  ...
  content::WebContents* web_contents = GetActiveWebContents();
  auto url = web_contents->GetLastCommittedURL(); // Determined at click-time
  ...
  SitePermissionsHelper permissions_helper(profile);
  permissions_helper.UpdateSiteAccess(*extension, web_contents, new_site_access);
}

The UpdateSiteAccess method in extensions/browser/permissions/site_permissions_helper.cc also re-queries web_contents->GetLastCommittedURL() to perform the actual permission modification.

The Extensions menu does not close automatically when a navigation occurs. Instead, it observes DidFinishNavigation and triggers a UI update via ExtensionsMenuDelegateDesktop::OnPageNavigation to reflect the state of the new page. However, there is no validation that the origin displayed to the user when they initiated the click matches the origin for which the permission is ultimately granted.

Potential Attack Scenario

  1. A user installs a malicious extension that requests broad host permissions (e.g., <all_urls>) but currently has those permissions withheld (Runtime Host Permissions enabled).
  2. The user visits an attacker-controlled site (e.g., https://attacker.example).
  3. The user opens the Extensions menu. The menu correctly shows the extension’s site access as “Off” for the current origin.
  4. The attacker page detects the menu interaction (e.g., via window.blur) and initiates a top-level navigation to a sensitive site (e.g., https://victim.example).
  5. The user clicks the toggle to grant site access.
  6. If the navigation to victim.example commits on the UI thread just before the click event is processed by GrantSiteAccess, the code will read the new sensitive URL and grant the extension persistent host permissions for victim.example instead of attacker.example.

Impact

An attacker can obtain durable host permissions for a sensitive origin without informed user consent. This allows a malicious extension to inject content scripts, read cookies, and access sensitive data on the victim origin indefinitely.

Suggested Fix

The Extensions menu should bind the site-access action to the origin that was active when the UI was rendered or when the user initiated the click. ExtensionsMenuViewModel::GrantSiteAccess should accept an url::Origin parameter representing the intended target, and the implementation should verify that this origin still matches the current state of the WebContents before proceeding with the grant.

Note: These steps are suggested based on source code analysis; our current environment does not support running a functional Proof of Concept.

Evaluated with Chrome root at commit: 1a8d40fc44df2088d5945c0bf53584038aa1614a

Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:

  • If you are familiar with the severity guidelines, you may adjust the severity.
  • If this is a false positive, and there’s no work to be done, please close as WAI.
  • If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.

Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.

View on issue tracker
On This Page