From c0e115725023ce4352650e22ed2c25baab0d1cfc Mon Sep 17 00:00:00 2001
From: James Cook <[email protected]>
Date: Tue, 26 May 2026 16:04:58 -0700
Subject: [PATCH] extensions: Fix bypass of user site restrictions in pageCapture API

PermissionData::CanCaptureVisiblePage(), when called from the
pageCapture API with CaptureRequirement::kPageCapture was not checking
if the user had blocked extension access to the page to capture.

Refactor out an IsUrlBlockedByUser() method and call it to check if
the URL should be allowed. We can't use the existing PermissionData
methods like GetPageAccess() or CanRunOnPage() because those would
require the extension to declare host_permissions, and that's not
part of the API contract for chrome.pageCapture.

Given that the risk of this defect is low (an extension could capture
an MHTML version of a page), I don't think it's worth changing the API
contract to require host_permissions, since that would likely break
existing extensions using pageCapture.

A follow-up CL will remove some unnecessary ENABLE_EXTENSIONS
buildflag checks, as desktop Android now supports pageCapture. I left
them in for easier merging, if we need it.

Bug: 514503077
Test: added to unit_tests
Change-Id: I2a4fe17de851adc996c4535e4e2c56e2e59086fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7871735
Reviewed-by: Tim <[email protected]>
Commit-Queue: James Cook <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1636553}
---

diff --git a/chrome/common/extensions/permissions/permissions_data_unittest.cc b/chrome/common/extensions/permissions/permissions_data_unittest.cc
index 240a5ba..7a0a658d 100644
--- a/chrome/common/extensions/permissions/permissions_data_unittest.cc
+++ b/chrome/common/extensions/permissions/permissions_data_unittest.cc
@@ -1791,4 +1791,30 @@
   }
 }
 
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+// TODO(crbug.com/427298257): Enable on desktop Android.
+TEST_F(CaptureVisiblePageTest, PageCapture_UserBlockedURLs) {
+  // Allow per-host user restrictions.
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(
+      extensions_features::kExtensionsMenuAccessControl);
+
+  // Apply a user host restriction.
+  URLPattern blocked_url(URLPattern::SCHEME_ALL, "https://blocked.com/*");
+  int context_id = 8;
+  URLPatternSet blocked_patterns({blocked_url});
+  PermissionsData::SetUserHostRestrictions(
+      context_id, std::move(blocked_patterns), URLPatternSet());
+  page_capture().permissions_data()->SetContextId(context_id);
+
+  // The user restricted URL can't be captured.
+  EXPECT_FALSE(CanCapture(page_capture(), GURL("https://blocked.com"),
+                          extensions::CaptureRequirement::kPageCapture));
+
+  // An arbitrary URL can be captured.
+  EXPECT_TRUE(CanCapture(page_capture(), GURL("https://allowed.com/"),
+                         extensions::CaptureRequirement::kPageCapture));
+}
+#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
+
 }  // namespace extensions
diff --git a/extensions/common/permissions/permissions_data.cc b/extensions/common/permissions/permissions_data.cc
index a7fe00a7..8f4206e 100644
--- a/extensions/common/permissions/permissions_data.cc
+++ b/extensions/common/permissions/permissions_data.cc
@@ -34,6 +34,8 @@
 
 namespace {
 
+constexpr char kErrorBlocked[] = "Blocked";
+
 PermissionsData::PolicyDelegate* g_policy_delegate = nullptr;
 
 struct URLPatternAccessSet {
@@ -529,6 +531,17 @@
       return false;
     }
 
+    // We can't use GetPageAccess() here because that would require developers
+    // using the pageCapture API to declare host_permissions in their manifest,
+    // and that's not part of the API spec. In lieu of that, check for URLs the
+    // user has specifically disallowed. https://crbug.com/514503077
+    if (IsUrlBlockedByUser(origin_url)) {
+      if (error) {
+        *error = kErrorBlocked;
+      }
+      return false;
+    }
+
     // If the URL is a typical web URL, the pageCapture permission is
     // sufficient.
     if ((origin_url.SchemeIs(url::kHttpScheme) ||
@@ -611,6 +624,30 @@
          !policy_allowed_hosts_unsafe_.MatchesURL(url);
 }
 
+bool PermissionsData::IsUrlBlockedByUser(const GURL& document_url) const {
+  // Only applies when per-URL extension blocking is enabled.
+  if (!base::FeatureList::IsEnabled(
+          extensions_features::kExtensionsMenuAccessControl)) {
+    return false;
+  }
+
+  if (!context_id_ || location_ == mojom::ManifestLocation::kComponent ||
+      Manifest::IsPolicyLocation(location_)) {
+    return false;
+  }
+
+  base::AutoLock lock(GetContextPermissionsLock());
+  auto& context_permissions = GetContextPermissions(*context_id_);
+  // Check if the host is restricted by the user. `allowed_hosts` takes
+  // precedent over `blocked_hosts`. Note that, today, PermissionsManager
+  // ensures there's no overlap, but this will change if/when
+  // PermissionsManager uses URLPatterns instead of origins.
+  return context_permissions.user_restrictions.blocked_hosts.MatchesURL(
+             document_url) &&
+         !context_permissions.user_restrictions.allowed_hosts.MatchesURL(
+             document_url);
+}
+
 PermissionsData::PageAccess PermissionsData::CanRunOnPage(
     const GURL& document_url,
     const URLPatternSet& permitted_url_patterns,
@@ -628,28 +665,14 @@
   if (IsRestrictedUrl(document_url, error))
     return PageAccess::kDenied;
 
-  if (base::FeatureList::IsEnabled(
-          extensions_features::kExtensionsMenuAccessControl) &&
-      context_id_ && location_ != mojom::ManifestLocation::kComponent &&
-      !Manifest::IsPolicyLocation(location_)) {
-    base::AutoLock lock(GetContextPermissionsLock());
-    auto& context_permissions = GetContextPermissions(*context_id_);
-    // Check if the host is restricted by the user. `allowed_hosts` takes
-    // precedent over `blocked_hosts`. Note that, today, PermissionsManager
-    // ensures there's no overlap, but this will change if/when
-    // PermissionsManager uses URLPatterns instead of origins.
-    if (context_permissions.user_restrictions.blocked_hosts.MatchesURL(
-            document_url) &&
-        !context_permissions.user_restrictions.allowed_hosts.MatchesURL(
-            document_url)) {
-      if (error) {
-        // TODO(crbug.com/40803363): What level of information should
-        // we specify here? Policy host restrictions pass a descriptive error
-        // back to the extension; is there any harm in doing so?
-        *error = "Blocked";
-      }
-      return PageAccess::kDenied;
+  if (IsUrlBlockedByUser(document_url)) {
+    if (error) {
+      // TODO(crbug.com/40803363): What level of information should
+      // we specify here? Policy host restrictions pass a descriptive error
+      // back to the extension; is there any harm in doing so?
+      *error = kErrorBlocked;
     }
+    return PageAccess::kDenied;
   }
 
   if (tab_url_patterns && tab_url_patterns->MatchesURL(document_url))
diff --git a/extensions/common/permissions/permissions_data.h b/extensions/common/permissions/permissions_data.h
index 49f77a9..cd222fe 100644
--- a/extensions/common/permissions/permissions_data.h
+++ b/extensions/common/permissions/permissions_data.h
@@ -208,6 +208,11 @@
                            int tab_id,
                            std::string* error) const;
 
+  // Returns true if there's a user host restriction that blocks `document_url`,
+  // unless there is one that explicitly allows it. Returns false if feature
+  // kExtensionsMenuAccessControl is not enabled.
+  bool IsUrlBlockedByUser(const GURL& document_url) const;
+
   // Returns true if the associated extension has permission to inject a
   // content script on the page.
   // If this returns false and `error` is non-NULL, `error` will be popualted

diff --git a/chrome/common/extensions/permissions/permissions_data_unittest.cc b/chrome/common/extensions/permissions/permissions_data_unittest.cc
index 240a5ba..7a0a658d 100644
--- a/chrome/common/extensions/permissions/permissions_data_unittest.cc
+++ b/chrome/common/extensions/permissions/permissions_data_unittest.cc
@@ -1791,4 +1791,30 @@
   }
 }
 
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+// TODO(crbug.com/427298257): Enable on desktop Android.
+TEST_F(CaptureVisiblePageTest, PageCapture_UserBlockedURLs) {
+  // Allow per-host user restrictions.
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(
+      extensions_features::kExtensionsMenuAccessControl);
+
+  // Apply a user host restriction.
+  URLPattern blocked_url(URLPattern::SCHEME_ALL, "https://blocked.com/*");
+  int context_id = 8;
+  URLPatternSet blocked_patterns({blocked_url});
+  PermissionsData::SetUserHostRestrictions(
+      context_id, std::move(blocked_patterns), URLPatternSet());
+  page_capture().permissions_data()->SetContextId(context_id);
+
+  // The user restricted URL can't be captured.
+  EXPECT_FALSE(CanCapture(page_capture(), GURL("https://blocked.com"),
+                          extensions::CaptureRequirement::kPageCapture));
+
+  // An arbitrary URL can be captured.
+  EXPECT_TRUE(CanCapture(page_capture(), GURL("https://allowed.com/"),
+                         extensions::CaptureRequirement::kPageCapture));
+}
+#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
+
 }  // namespace extensions