Medium chrome Memory Corruption 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactMemory corruption in Extensions
DescriptionMemory corruption in Extensions
ComponentExtensions
Sub-componentExtensions
Bug ClassMemory Corruption
Tracker501370810
Fix commit95159a96a630 (chromium/src) +154/-22
CISA KEVNot listed
CreditedGoogle
Disclosed2026-10-20

Changed Functions

FunctionChangeNotes
ExtensionDisabledGlobalErrorTest
chrome/browser/extensions/extension_disabled_ui_browsertest.cc
modified
ASSERT_TRUE
chrome/browser/extensions/extension_disabled_ui_browsertest.cc
modified

Files Changed

  • chrome/browser/extensions/extension_disabled_ui_browsertest.cc
  • extensions/browser/extension_registrar_unittest.cc
  • chrome/browser/extensions/extension_disabled_ui_desktop.cc
  • extensions/browser/extension_registrar.cc
From 95159a96a6301655dc10e57015c4d46db850a703 Mon Sep 17 00:00:00 2001
From: Anh Mac <[email protected]>
Date: Wed, 16 Sep 2026 14:13:13 -0700
Subject: [PATCH] [Extensions] Dismiss stale permission error on version update while disabled

When an extension disabled due to a permission increase is updated, the
previous disabled global error could remain stale, and accepting it
could grant permissions for the updated version when the stale UI only
shows permissions for the outdated version.

This change:
- Removes ExtensionDisabledGlobalError upon OnExtensionWillBeInstalled so
stale error UI bubble is cleared when a permission update occurs.
- Defensively rejects GrantPermissionsAndEnableExtension calls in
ExtensionRegistrar if the extension version does not match the currently
installed version (e.g. user accepting a stale error bubble will be a
no-op).
- Adds test coverage for updating an extension while disabled and attempting
to grant permissions with a mismatched version.

Fixed: 501370810
Change-Id: Ief8fc245835edb9095664bea2f3dcaee0a6370c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8407375
Commit-Queue: Anh Mac <[email protected]>
Reviewed-by: Devlin Cronin <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1699258}
---
diff --git a/chrome/browser/extensions/extension_disabled_ui_desktop.cc b/chrome/browser/extensions/extension_disabled_ui_desktop.cc
index 95c83d2..8cd41f32 100644
--- a/chrome/browser/extensions/extension_disabled_ui_desktop.cc
+++ b/chrome/browser/extensions/extension_disabled_ui_desktop.cc
@@ -87,6 +87,10 @@
   void OnExtensionUninstalled(content::BrowserContext* browser_context,
                               const Extension* extension,
                               UninstallReason reason) override;
+  void OnExtensionWillBeInstalled(content::BrowserContext* browser_context,
+                                  const Extension* extension,
+                                  bool is_update,
+                                  const std::string& old_name) override;
   void OnShutdown(ExtensionRegistry* registry) override;
 
   void RemoveGlobalError();
@@ -283,6 +287,17 @@
   RemoveGlobalError();
 }
 
+void ExtensionDisabledGlobalError::OnExtensionWillBeInstalled(
+    content::BrowserContext* browser_context,
+    const Extension* extension,
+    bool is_update,
+    const std::string& old_name) {
+  if (extension->id() != extension_->id()) {
+    return;
+  }
+  RemoveGlobalError();
+}
+
 void ExtensionDisabledGlobalError::OnShutdown(ExtensionRegistry* registry) {
   DCHECK_EQ(ExtensionRegistry::Get(profile_), registry);
   registry_observation_.Reset();
diff --git a/extensions/browser/extension_registrar.cc b/extensions/browser/extension_registrar.cc
index 435fe81..cd8cfec 100644
--- a/extensions/browser/extension_registrar.cc
+++ b/extensions/browser/extension_registrar.cc
@@ -1014,6 +1014,12 @@
 
 void ExtensionRegistrar::GrantPermissionsAndEnableExtension(
     const Extension& extension) {
+  // Only proceed when the requested input extension still matches the installed
+  // version; otherwise, reject early.
+  const Extension* installed = registry_->GetInstalledExtension(extension.id());
+  if (!installed || installed->version() != extension.version()) {
+    return;
+  }
   delegate_->GrantActivePermissions(&extension);
   EnableExtension(extension.id());
 }
diff --git a/chrome/browser/extensions/extension_disabled_ui_browsertest.cc b/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
index 0a48544..4fb0f98 100644
--- a/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
+++ b/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
@@ -14,6 +14,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind.h"
+#include "base/test/run_until.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -43,7 +44,9 @@
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/mojom/api_permission_id.mojom.h"
 #include "extensions/common/permissions/permission_set.h"
+#include "extensions/common/permissions/permissions_data.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -53,6 +56,12 @@
 using extensions::ExtensionPrefs;
 using extensions::ExtensionSyncData;
 
+namespace {
+constexpr int kMaxExtensionInstallErrors = 101;
+constexpr int IDC_EXTENSION_INSTALL_ERROR_LAST =
+    IDC_EXTENSION_INSTALL_ERROR_FIRST + kMaxExtensionInstallErrors - 1;
+}  // namespace
+
 class ExtensionDisabledGlobalErrorTest
     : public extensions::ExtensionBrowserTest {
  protected:
@@ -79,11 +88,29 @@
         base::FilePath());
   }
 
+  int GetExtensionDisabledErrorCount(GlobalErrorService* service,
+                                     const std::u16string_view extension_name) {
+    DCHECK(!extension_name.empty());
+    DCHECK(service);
+    return std::ranges::count_if(service->errors(), [extension_name](
+                                                        GlobalError* error) {
+      return error->MenuItemCommandID() >= IDC_EXTENSION_INSTALL_ERROR_FIRST &&
+             error->MenuItemCommandID() <= IDC_EXTENSION_INSTALL_ERROR_LAST &&
+             error->MenuItemLabel().find(extension_name) != std::string::npos;
+    });
+  }
+
   // Returns the ExtensionDisabledGlobalError, if present.
   // Caution: currently only supports one error at a time.
   GlobalError* GetExtensionDisabledGlobalError() {
-    return GlobalErrorServiceFactory::GetForProfile(profile())->
-        GetGlobalErrorByMenuItemCommandID(IDC_EXTENSION_INSTALL_ERROR_FIRST);
+    auto* service = GlobalErrorServiceFactory::GetForProfile(profile());
+    for (GlobalError* error : service->errors()) {
+      if (error->MenuItemCommandID() >= IDC_EXTENSION_INSTALL_ERROR_FIRST &&
+          error->MenuItemCommandID() <= IDC_EXTENSION_INSTALL_ERROR_LAST) {
+        return error;
+      }
+    }
+    return nullptr;
   }
 
   // Install the initial version, which should happen just fine.
@@ -356,32 +383,13 @@
   EXPECT_TRUE(GetExtensionDisabledGlobalError());
 }
 
-namespace {
-
-constexpr int kMaxExtensionInstallErrors = 101;
-constexpr int IDC_EXTENSION_INSTALL_ERROR_LAST =
-    IDC_EXTENSION_INSTALL_ERROR_FIRST + kMaxExtensionInstallErrors - 1;
-
-int GetExtensionDisabledErrorCount(GlobalErrorService* service,
-                                   const std::u16string_view extension_name) {
-  DCHECK(!extension_name.empty());
-  DCHECK(service);
-  return std::ranges::count_if(service->errors(), [extension_name](
-                                                      GlobalError* error) {
-    return error->MenuItemCommandID() >= IDC_EXTENSION_INSTALL_ERROR_FIRST &&
-           error->MenuItemCommandID() <= IDC_EXTENSION_INSTALL_ERROR_LAST &&
-           error->MenuItemLabel().find(extension_name) != std::string::npos;
-  });
-}
-}  // namespace
-
 IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
                        AllErrorsRemovedWhenExtensionRemoved) {
   const Extension* extension = InstallIncreasingPermissionExtensionV1();
   ASSERT_TRUE(extension);
-  AddExtensionDisabledError(browser()->GetProfile(), extension, false);
   extension = UpdateIncreasingPermissionExtension(extension, path_v2_, -1);
   ASSERT_TRUE(extension);
+  AddExtensionDisabledError(browser()->GetProfile(), extension, false);
 
   const auto extension_name = base::UTF8ToUTF16(extension->name());
   auto* global_error_service =
@@ -401,3 +409,68 @@
   EXPECT_EQ(
       GetExtensionDisabledErrorCount(global_error_service, extension_name), 0);
 }
+
+IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
+                       UpdateWhileDisabled_StaleErrorReplacedOnUpdate) {
+  // Install v1
+  const Extension* extension = InstallIncreasingPermissionExtensionV1();
+  ASSERT_TRUE(extension);
+  auto* global_error_service =
+      GlobalErrorServiceFactory::GetForProfile(profile());
+  const std::string extension_id = extension->id();
+  const auto extension_name = base::UTF8ToUTF16(extension->name());
+  // Update to v2 (adds "tabs"). Extension gets disabled for permissions
+  // increase.
+  extension = UpdateIncreasingPermissionExtension(extension, path_v2_, -1);
+  ASSERT_TRUE(extension);
+  EXPECT_TRUE(
+      extension_registry()->disabled_extensions().Contains(extension_id));
+  EXPECT_EQ(
+      GetExtensionDisabledErrorCount(global_error_service, extension_name), 1);
+
+  // Update to v3 (adds "tabs" and "management") while still disabled. The
+  // stale v2 error was removed on update, leaving exactly 1 error for v3.
+  extension = UpdateIncreasingPermissionExtension(extension, path_v3_, 0);
+  ASSERT_TRUE(extension);
+  EXPECT_EQ(
+      GetExtensionDisabledErrorCount(global_error_service, extension_name), 1);
+
+  GlobalErrorWithStandardBubble* error =
+      static_cast<GlobalErrorWithStandardBubble*>(
+          GetExtensionDisabledGlobalError());
+  ASSERT_TRUE(error);
+  error->BubbleViewAcceptButtonPressed(browser());
+  ASSERT_TRUE(base::test::RunUntil([&]() {
+    return extension_registry()->enabled_extensions().Contains(extension_id);
+  }));
+
+  // Verify the extension is granted and enabled with the permissions from v3.
+  const Extension* enabled_extension =
+      extension_registry()->enabled_extensions().GetByID(extension_id);
+  ASSERT_TRUE(enabled_extension);
+  EXPECT_EQ(enabled_extension->version().GetString(), "3");
+
+  std::unique_ptr<const extensions::PermissionSet> granted_permissions =
+      ExtensionPrefs::Get(profile())->GetGrantedPermissions(extension_id);
+  ASSERT_TRUE(granted_permissions);
+  EXPECT_TRUE(granted_permissions->HasAPIPermission(
+      extensions::mojom::APIPermissionID::kTab));
+  EXPECT_TRUE(granted_permissions->HasAPIPermission(
+      extensions::mojom::APIPermissionID::kManagement));
+  EXPECT_TRUE(enabled_extension->permissions_data()->HasAPIPermission(
+      extensions::mojom::APIPermissionID::kTab));
+  EXPECT_TRUE(enabled_extension->permissions_data()->HasAPIPermission(
+      extensions::mojom::APIPermissionID::kManagement));
+}
+
+// Tests that installing an unrelated extension does not remove an active
+// ExtensionDisabledGlobalError for a disabled extension.
+IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
+                       UnrelatedExtensionInstallDoesNotRemoveError) {
+  const Extension* extension = InstallAndUpdateIncreasingPermissionsExtension();
+  ASSERT_TRUE(extension);
+  ASSERT_TRUE(GetExtensionDisabledGlobalError());
+
+  ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("simple_with_file")));
+  EXPECT_TRUE(GetExtensionDisabledGlobalError());
+}
diff --git a/extensions/browser/extension_registrar_unittest.cc b/extensions/browser/extension_registrar_unittest.cc
index d102e0dd..a59c553e 100644
--- a/extensions/browser/extension_registrar_unittest.cc
+++ b/extensions/browser/extension_registrar_unittest.cc
@@ -24,6 +24,9 @@
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
+#include "extensions/common/mojom/api_permission_id.mojom.h"
+#include "extensions/common/permissions/permission_set.h"
+#include "extensions/common/permissions/permissions_data.h"
 #include "extensions/common/switches.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -630,4 +633,39 @@
                   .empty());
 }
 
+TEST_F(ExtensionRegistrarTest,
+       GrantPermissionsAndEnableExtension_MismatchedVersion) {
+  AddDisabledExtension();
+
+  // Mock the delegate so that it grants the extension's active permissions in
+  // ExtensionPrefs when invoked.
+  ON_CALL(*delegate(), GrantActivePermissions(_))
+      .WillByDefault([this](const Extension* ext) {
+        ExtensionPrefs::Get(browser_context())
+            ->AddGrantedPermissions(
+                ext->id(), ext->permissions_data()->active_permissions());
+      });
+
+  scoped_refptr<const Extension> extension_different_version =
+      ExtensionBuilder("extension")
+          .SetVersion("2.0")
+          .AddAPIPermission("bookmarks")
+          .SetID(extension()->id())
+          .Build();
+
+  // Assert that GrantActivePermissions is not called.
+  EXPECT_CALL(*delegate(), GrantActivePermissions(_)).Times(0);
+  registrar()->GrantPermissionsAndEnableExtension(*extension_different_version);
+
+  ExpectInSet(ExtensionRegistry::DISABLED);
+
+  // Verify that the extension was not granted the newly added permissions when
+  // GrantPermissionsAndEnableExtension was called with a mismatched version.
+  const Extension* installed = ExtensionRegistry::Get(browser_context())
+                                   ->GetInstalledExtension(extension()->id());
+  ASSERT_TRUE(installed);
+  EXPECT_FALSE(installed->permissions_data()->HasAPIPermission(
+      mojom::APIPermissionID::kBookmark));
+}
+
 }  // namespace extensions
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/chrome/browser/extensions/extension_disabled_ui_browsertest.cc b/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
index 0a48544..4fb0f98 100644
--- a/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
+++ b/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
@@ -14,6 +14,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind.h"
+#include "base/test/run_until.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -43,7 +44,9 @@
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/mojom/api_permission_id.mojom.h"
 #include "extensions/common/permissions/permission_set.h"
+#include "extensions/common/permissions/permissions_data.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -53,6 +56,12 @@
 using extensions::ExtensionPrefs;
 using extensions::ExtensionSyncData;
 
+namespace {
+constexpr int kMaxExtensionInstallErrors = 101;
+constexpr int IDC_EXTENSION_INSTALL_ERROR_LAST =
+    IDC_EXTENSION_INSTALL_ERROR_FIRST + kMaxExtensionInstallErrors - 1;
+}  // namespace
+
 class ExtensionDisabledGlobalErrorTest
     : public extensions::ExtensionBrowserTest {
  protected:
@@ -79,11 +88,29 @@
         base::FilePath());
   }
 
+  int GetExtensionDisabledErrorCount(GlobalErrorService* service,
+                                     const std::u16string_view extension_name) {
+    DCHECK(!extension_name.empty());
+    DCHECK(service);
+    return std::ranges::count_if(service->errors(), [extension_name](
+                                                        GlobalError* error) {
+      return error->MenuItemCommandID() >= IDC_EXTENSION_INSTALL_ERROR_FIRST &&
+             error->MenuItemCommandID() <= IDC_EXTENSION_INSTALL_ERROR_LAST &&
+             error->MenuItemLabel().find(extension_name) != std::string::npos;
+    });
+  }
+
   // Returns the ExtensionDisabledGlobalError, if present.
   // Caution: currently only supports one error at a time.
   GlobalError* GetExtensionDisabledGlobalError() {
-    return GlobalErrorServiceFactory::GetForProfile(profile())->
-        GetGlobalErrorByMenuItemCommandID(IDC_EXTENSION_INSTALL_ERROR_FIRST);
+    auto* service = GlobalErrorServiceFactory::GetForProfile(profile());
+    for (GlobalError* error : service->errors()) {
+      if (error->MenuItemCommandID() >= IDC_EXTENSION_INSTALL_ERROR_FIRST &&
+          error->MenuItemCommandID() <= IDC_EXTENSION_INSTALL_ERROR_LAST) {
+        return error;
+      }
+    }
+    return nullptr;
   }
 
   // Install the initial version, which should happen just fine.
@@ -356,32 +383,13 @@
   EXPECT_TRUE(GetExtensionDisabledGlobalError());
 }
 
-namespace {
-
-constexpr int kMaxExtensionInstallErrors = 101;
-constexpr int IDC_EXTENSION_INSTALL_ERROR_LAST =
-    IDC_EXTENSION_INSTALL_ERROR_FIRST + kMaxExtensionInstallErrors - 1;
-
-int GetExtensionDisabledErrorCount(GlobalErrorService* service,
-                                   const std::u16string_view extension_name) {
-  DCHECK(!extension_name.empty());
-  DCHECK(service);
-  return std::ranges::count_if(service->errors(), [extension_name](
-                                                      GlobalError* error) {
-    return error->MenuItemCommandID() >= IDC_EXTENSION_INSTALL_ERROR_FIRST &&
-           error->MenuItemCommandID() <= IDC_EXTENSION_INSTALL_ERROR_LAST &&
-           error->MenuItemLabel().find(extension_name) != std::string::npos;
-  });
-}
-}  // namespace
-
 IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
                        AllErrorsRemovedWhenExtensionRemoved) {
   const Extension* extension = InstallIncreasingPermissionExtensionV1();
   ASSERT_TRUE(extension);
-  AddExtensionDisabledError(browser()->GetProfile(), extension, false);
   extension = UpdateIncreasingPermissionExtension(extension, path_v2_, -1);
   ASSERT_TRUE(extension);
+  AddExtensionDisabledError(browser()->GetProfile(), extension, false);
 
   const auto extension_name = base::UTF8ToUTF16(extension->name());
   auto* global_error_service =
@@ -401,3 +409,68 @@
   EXPECT_EQ(
       GetExtensionDisabledErrorCount(global_error_service, extension_name), 0);
 }
+
+IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
+                       UpdateWhileDisabled_StaleErrorReplacedOnUpdate) {
+  // Install v1
+  const Extension* extension = InstallIncreasingPermissionExtensionV1();
+  ASSERT_TRUE(extension);
+  auto* global_error_service =
+      GlobalErrorServiceFactory::GetForProfile(profile());
+  const std::string extension_id = extension->id();
+  const auto extension_name = base::UTF8ToUTF16(extension->name());
+  // Update to v2 (adds "tabs"). Extension gets disabled for permissions
+  // increase.
+  extension = UpdateIncreasingPermissionExtension(extension, path_v2_, -1);
+  ASSERT_TRUE(extension);
+  EXPECT_TRUE(
+      extension_registry()->disabled_extensions().Contains(extension_id));
+  EXPECT_EQ(
+      GetExtensionDisabledErrorCount(global_error_service, extension_name), 1);
+
+  // Update to v3 (adds "tabs" and "management") while still disabled. The
+  // stale v2 error was removed on update, leaving exactly 1 error for v3.
+  extension = UpdateIncreasingPermissionExtension(extension, path_v3_, 0);
+  ASSERT_TRUE(extension);
+  EXPECT_EQ(
+      GetExtensionDisabledErrorCount(global_error_service, extension_name), 1);
+
+  GlobalErrorWithStandardBubble* error =
+      static_cast<GlobalErrorWithStandardBubble*>(
+          GetExtensionDisabledGlobalError());
+  ASSERT_TRUE(error);
+  error->BubbleViewAcceptButtonPressed(browser());
+  ASSERT_TRUE(base::test::RunUntil([&]() {
+    return extension_registry()->enabled_extensions().Contains(extension_id);
+  }));
+
+  // Verify the extension is granted and enabled with the permissions from v3.
+  const Extension* enabled_extension =
+      extension_registry()->enabled_extensions().GetByID(extension_id);
+  ASSERT_TRUE(enabled_extension);
+  EXPECT_EQ(enabled_extension->version().GetString(), "3");
+
+  std::unique_ptr<const extensions::PermissionSet> granted_permissions =
+      ExtensionPrefs::Get(profile())->GetGrantedPermissions(extension_id);
+  ASSERT_TRUE(granted_permissions);
+  EXPECT_TRUE(granted_permissions->HasAPIPermission(
+      extensions::mojom::APIPermissionID::kTab));
+  EXPECT_TRUE(granted_permissions->HasAPIPermission(
+      extensions::mojom::APIPermissionID::kManagement));
+  EXPECT_TRUE(enabled_extension->permissions_data()->HasAPIPermission(
+      extensions::mojom::APIPermissionID::kTab));
+  EXPECT_TRUE(enabled_extension->permissions_data()->HasAPIPermission(
+      extensions::mojom::APIPermissionID::kManagement));
+}
+
+// Tests that installing an unrelated extension does not remove an active
+// ExtensionDisabledGlobalError for a disabled extension.
+IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
+                       UnrelatedExtensionInstallDoesNotRemoveError) {
+  const Extension* extension = InstallAndUpdateIncreasingPermissionsExtension();
+  ASSERT_TRUE(extension);
+  ASSERT_TRUE(GetExtensionDisabledGlobalError());
+
+  ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("simple_with_file")));
+  EXPECT_TRUE(GetExtensionDisabledGlobalError());
+}
diff --git a/extensions/browser/extension_registrar_unittest.cc b/extensions/browser/extension_registrar_unittest.cc
index d102e0dd..a59c553e 100644
--- a/extensions/browser/extension_registrar_unittest.cc
+++ b/extensions/browser/extension_registrar_unittest.cc
@@ -24,6 +24,9 @@
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
+#include "extensions/common/mojom/api_permission_id.mojom.h"
+#include "extensions/common/permissions/permission_set.h"
+#include "extensions/common/permissions/permissions_data.h"
 #include "extensions/common/switches.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -630,4 +633,39 @@
                   .empty());
 }
 
+TEST_F(ExtensionRegistrarTest,
+       GrantPermissionsAndEnableExtension_MismatchedVersion) {
+  AddDisabledExtension();
+
+  // Mock the delegate so that it grants the extension's active permissions in
+  // ExtensionPrefs when invoked.
+  ON_CALL(*delegate(), GrantActivePermissions(_))
+      .WillByDefault([this](const Extension* ext) {
+        ExtensionPrefs::Get(browser_context())
+            ->AddGrantedPermissions(
+                ext->id(), ext->permissions_data()->active_permissions());
+      });
+
+  scoped_refptr<const Extension> extension_different_version =
+      ExtensionBuilder("extension")
+          .SetVersion("2.0")
+          .AddAPIPermission("bookmarks")
+          .SetID(extension()->id())
+          .Build();
+
+  // Assert that GrantActivePermissions is not called.
+  EXPECT_CALL(*delegate(), GrantActivePermissions(_)).Times(0);
+  registrar()->GrantPermissionsAndEnableExtension(*extension_different_version);
+
+  ExpectInSet(ExtensionRegistry::DISABLED);
+
+  // Verify that the extension was not granted the newly added permissions when
+  // GrantPermissionsAndEnableExtension was called with a mismatched version.
+  const Extension* installed = ExtensionRegistry::Get(browser_context())
+                                   ->GetInstalledExtension(extension()->id());
+  ASSERT_TRUE(installed);
+  EXPECT_FALSE(installed->permissions_data()->HasAPIPermission(
+      mojom::APIPermissionID::kBookmark));
+}
+
 }  // namespace extensions
Loading diff…

Original Bug Report

reported by [email protected]

TOCTOU in ExtensionDisabledGlobalError allows unreviewed permission escalation

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 without the Chrome Security team.

Overview: A potential Time-of-Check Time-of-Use (TOCTOU) vulnerability exists in the extension re-enablement UI. If a disabled extension updates to a version with more permissions, a stale UI prompt for the older version can persist. Accepting this stale prompt activates the newer version with its full, unreviewed permissions.

Affected files:

  • chrome/browser/extensions/extension_disabled_ui_desktop.cc
  • extensions/browser/extension_registrar.cc

Estimated timestamp from git blame: 2025-02-28

Summary

A potential logic flaw in how Chrome handles extension re-enablement could allow a newer version of an extension to run with unreviewed permissions if it updates while already disabled for a permission increase.

Note: The following behavior is based on static analysis by our tooling agent, which does not currently have the ability to run a live proof-of-concept. Thus, this is described as a potential vulnerability.

Technical Details

When an extension is disabled due to a permission increase (e.g., v1.0.1), an ExtensionDisabledGlobalError is instantiated (chrome/browser/extensions/extension_disabled_ui_desktop.cc). This object stores a scoped_refptr<const Extension> referencing the specific v1.0.1 object and registers itself with the GlobalErrorService.

If the extension auto-updates to v1.1 while disabled, ExtensionRegistrar removes v1.0.1 from the disabled registry and adds v1.1. However, ExtensionDisabledGlobalError only observes OnExtensionLoaded and OnExtensionUninstalled. It does not handle OnExtensionUnloaded during updates. Consequently, the stale error referencing v1.0.1 remains in the GlobalErrorService alongside a newly generated error for v1.1.

When the user clicks the global error menu item, GlobalErrorService returns the earliest registered error (the one for v1.0.1). The UI displays the permissions for v1.0.1. Upon acceptance, ExtensionDisabledGlobalError::BubbleViewAcceptButtonPressed posts a task calling ExtensionRegistrar::GrantPermissionsAndEnableExtension with the stale v1.0.1 scoped_refptr.

GrantPermissionsAndEnableExtension (extensions/browser/extension_registrar.cc) handles this inconsistently:

  1. It calls delegate_->GrantActivePermissions(&extension), writing v1.0.1’s permissions to the granted preferences.
  2. It then calls EnableExtension(extension.id()), which looks up the extension by ID from the disabled registry. This retrieves the newer v1.1 object.
  3. EnableExtension unconditionally clears the DISABLE_PERMISSIONS_INCREASE flag and activates v1.1.

Because v1.1’s PermissionsData was initialized with its full required permissions during its load, it now runs with elevated privileges that the user never saw or accepted (e.g., <all_urls>). Although it will be re-disabled on the next browser restart (because prefs only hold v1.0.1’s permissions), the attacker gains full unreviewed permissions for the current session.

Potential Attacker Steps

  1. An attacker publishes a benign extension (v1.0) which is installed by the user.
  2. The attacker publishes v1.0.1, adding a minor permission (e.g., storage). Chrome auto-updates, disables the extension, and generates the UI error.
  3. While the extension is disabled, the attacker publishes v1.1, adding a critical permission (e.g., <all_urls>). Chrome auto-updates the disabled extension to v1.1.
  4. The user clicks the stale error notification in the Chrome menu and accepts the prompt, reviewing only the storage permission.
  5. Chrome activates v1.1, granting it <all_urls> access.

Suggested Fix

There are a few ways to resolve this state desynchronization:

  1. Update ExtensionDisabledGlobalError to observe OnExtensionUnloaded and remove itself if the extension is unloaded due to an update (UnloadedExtensionReason::UPDATE).
  2. Change ExtensionDisabledGlobalError to store only the ExtensionId rather than a scoped_refptr, ensuring it always fetches the latest extension object from the registry before displaying the prompt or enabling the extension.
  3. In ExtensionRegistrar::GrantPermissionsAndEnableExtension, verify that the scoped_refptr passed matches the current object in the disabled registry before proceeding with enablement.

Evaluated with Chrome root at commit: 096fc8fdbfacf2546485756d03f160a3d04fcc9b


Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; 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