From 274e7913ec91f1be04e068a576bbd2a9784a9672 Mon Sep 17 00:00:00 2001
From: Thomas Nguyen <[email protected]>
Date: Tue, 08 Sep 2026 01:57:45 -0700
Subject: [PATCH] [CE] Verify embedded permission registration and activation

Ensure embedded permission requests originate from valid embedded
permission controls and possess legitimate user activation.

Previously, embedded permission requests could bypass some of these
checks, and PermissionRequestDescription hardcoded the user gesture
flag to true. This change introduces validation against the
EmbeddedPermissionControlChecker to verify that an allowed embedded
control is registered on the page.

This change also refactors some reusing code with new shared helpers,

Fixed: 511776800,498375898
Change-Id: I37cba0e67935407d7e766958cc038ff0bc2f3a1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8366644
Reviewed-by: Camille Lamy <[email protected]>
Commit-Queue: Camille Lamy <[email protected]>
Reviewed-by: Ravjit Uppal <[email protected]>
Auto-Submit: Thomas Nguyen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1693633}
---
diff --git a/content/browser/permissions/embedded_permission_control_checker.cc b/content/browser/permissions/embedded_permission_control_checker.cc
index ee99f758..7cca1590 100644
--- a/content/browser/permissions/embedded_permission_control_checker.cc
+++ b/content/browser/permissions/embedded_permission_control_checker.cc
@@ -67,6 +67,21 @@
   }
 }
 
+bool EmbeddedPermissionControlChecker::HasPageEmbeddedPermission(
+    Source source,
+    const std::set<PermissionName>& permissions) const {
+  if (base::FeatureList::IsEnabled(
+          blink::features::kBypassPepcSecurityForTesting)) {
+    return true;
+  }
+  ClientKey key(source, permissions);
+  auto client_map_it = client_map_.find(key);
+  if (client_map_it == client_map_.end() || client_map_it->second.empty()) {
+    return false;
+  }
+  return true;
+}
+
 PAGE_USER_DATA_KEY_IMPL(EmbeddedPermissionControlChecker);
 
 void EmbeddedPermissionControlChecker::OnClientDisconnect(
@@ -87,6 +102,11 @@
     }
   }
 
+  if (queue.empty()) {
+    client_map_.erase(client_map_it);
+    return;
+  }
+
   for (auto it = queue.begin();
        it != queue.end() && std::distance(queue.begin(), it) < max_elements;
        ++it) {
diff --git a/content/browser/permissions/embedded_permission_control_checker.h b/content/browser/permissions/embedded_permission_control_checker.h
index 979ea1cd..36e145c9 100644
--- a/content/browser/permissions/embedded_permission_control_checker.h
+++ b/content/browser/permissions/embedded_permission_control_checker.h
@@ -63,6 +63,12 @@
           pending_client,
       RegisterPageEmbeddedPermissionCallback callback);
 
+  // Checks if the page currently has an allowed embedded permission control
+  // registered for the given source and permissions.
+  bool HasPageEmbeddedPermission(
+      Source source,
+      const std::set<blink::mojom::PermissionName>& permissions) const;
+
  private:
   friend class content::PageUserData<EmbeddedPermissionControlChecker>;
 
diff --git a/content/browser/permissions/permission_service_impl.cc b/content/browser/permissions/permission_service_impl.cc
index 062fb40..f656a54 100644
--- a/content/browser/permissions/permission_service_impl.cc
+++ b/content/browser/permissions/permission_service_impl.cc
@@ -8,6 +8,7 @@
 
 #include <algorithm>
 #include <memory>
+#include <optional>
 #include <ranges>
 #include <set>
 #include <utility>
@@ -184,6 +185,57 @@
   return true;
 }
 
+const base::Feature& GetRequiredFeatureForPageEmbeddedPermission(
+    const blink::mojom::EmbeddedPermissionRequestDescriptorPtr& descriptor) {
+  switch (descriptor->detail->which()) {
+    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
+        kGeolocation:
+      return blink::features::kGeolocationElement;
+    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
+        kInstall:
+      return blink::features::kInstallElement;
+    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
+        kUserMedia:
+      return blink::features::kUserMediaElement;
+  }
+}
+
+EmbeddedPermissionControlChecker::Source GetCheckerSourceForDescriptor(
+    const blink::mojom::EmbeddedPermissionRequestDescriptorPtr& descriptor) {
+  switch (descriptor->detail->which()) {
+    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
+        kGeolocation:
+      return EmbeddedPermissionControlChecker::Source::kGeolocationElement;
+    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
+        kInstall:
+      return EmbeddedPermissionControlChecker::Source::kInstallElement;
+    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
+        kUserMedia:
+      return EmbeddedPermissionControlChecker::Source::kUserMediaElement;
+  }
+}
+
+std::optional<std::set<PermissionName>> GetValidEmbeddedPermissionNames(
+    const std::vector<PermissionDescriptorPtr>& permissions,
+    const blink::mojom::EmbeddedPermissionRequestDescriptorPtr& descriptor) {
+  if (!CheckPageEmbeddedPermissionTypes(permissions, descriptor)) {
+    return std::nullopt;
+  }
+  std::set<PermissionName> permission_names;
+  for (const auto& permission : permissions) {
+    if (!ValidatePermissionDescriptor(permission)) {
+      return std::nullopt;
+    }
+    // Check for duplicates, and ensure we're only handling permission types
+    // which can be accessed through embedded controls:
+    if (PermissionUtil::IsEmbeddablePermission(permission) &&
+        !permission_names.insert(permission->name).second) {
+      return std::nullopt;
+    }
+  }
+  return permission_names;
+}
+
 }  // anonymous namespace
 
 class PermissionServiceImpl::PendingRequest {
@@ -222,37 +274,22 @@
     std::vector<PermissionDescriptorPtr> permissions,
     blink::mojom::EmbeddedPermissionRequestDescriptorPtr descriptor,
     mojo::PendingRemote<EmbeddedPermissionControlClient> observer) {
-  switch (descriptor->detail->which()) {
-    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
-        kGeolocation:
-      if (!base::FeatureList::IsEnabled(blink::features::kGeolocationElement)) {
-        bad_message::ReceivedBadMessage(
-            context_->render_frame_host()->GetProcess(),
-            bad_message::PSI_REGISTER_PERMISSION_ELEMENT_WITHOUT_FEATURE);
-        return;
-      }
-      break;
-    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
-        kInstall:
-      if (!base::FeatureList::IsEnabled(blink::features::kInstallElement)) {
-        bad_message::ReceivedBadMessage(
-            context_->render_frame_host()->GetProcess(),
-            bad_message::PSI_REGISTER_PERMISSION_ELEMENT_WITHOUT_FEATURE);
-        return;
-      }
-      break;
-    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
-        kUserMedia:
-      if (!base::FeatureList::IsEnabled(blink::features::kUserMediaElement)) {
-        bad_message::ReceivedBadMessage(
-            context_->render_frame_host()->GetProcess(),
-            bad_message::PSI_REGISTER_PERMISSION_ELEMENT_WITHOUT_FEATURE);
-        return;
-      }
-      break;
+  if (!context_->render_frame_host()) {
+    return;
   }
 
-  if (!CheckPageEmbeddedPermissionTypes(permissions, descriptor)) {
+  const base::Feature& required_feature =
+      GetRequiredFeatureForPageEmbeddedPermission(descriptor);
+  if (!base::FeatureList::IsEnabled(required_feature)) {
+    bad_message::ReceivedBadMessage(
+        context_->render_frame_host()->GetProcess(),
+        bad_message::PSI_REGISTER_PERMISSION_ELEMENT_WITHOUT_FEATURE);
+    return;
+  }
+
+  std::optional<std::set<PermissionName>> permission_names =
+      GetValidEmbeddedPermissionNames(permissions, descriptor);
+  if (!permission_names) {
     ReceivedBadMessage();
     return;
   }
@@ -262,39 +299,10 @@
   CHECK(web_contents);
   auto* checker = EmbeddedPermissionControlChecker::GetOrCreateForPage(
       web_contents->GetPrimaryPage());
-  std::set<PermissionName> permission_names;
-  for (const auto& permission : permissions) {
-    if (!ValidatePermissionDescriptor(permission)) {
-      ReceivedBadMessage();
-      return;
-    }
-    // Check for duplicates, and ensure we're only handling permission types
-    // which can be accessed through embedded controls:
-    if (PermissionUtil::IsEmbeddablePermission(permission) &&
-        !permission_names.insert(permission->name).second) {
-      ReceivedBadMessage();
-      return;
-    }
-  }
 
-  EmbeddedPermissionControlChecker::Source source =
-      EmbeddedPermissionControlChecker::Source::kUserMediaElement;
-  switch (descriptor->detail->which()) {
-    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
-        kGeolocation:
-      source = EmbeddedPermissionControlChecker::Source::kGeolocationElement;
-      break;
-    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
-        kInstall:
-      source = EmbeddedPermissionControlChecker::Source::kInstallElement;
-      break;
-    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
-        kUserMedia:
-      source = EmbeddedPermissionControlChecker::Source::kUserMediaElement;
-      break;
-  }
   checker->CheckPageEmbeddedPermission(
-      source, std::move(permission_names), std::move(observer),
+      GetCheckerSourceForDescriptor(descriptor), std::move(*permission_names),
+      std::move(observer),
       base::BindOnce(
           &PermissionServiceImpl::OnPageEmbeddedPermissionControlRegistered,
           weak_factory_.GetWeakPtr(), std::move(permissions)));
@@ -328,34 +336,14 @@
     std::vector<PermissionDescriptorPtr> permissions,
     EmbeddedPermissionRequestDescriptorPtr descriptor,
     RequestPageEmbeddedPermissionCallback callback) {
-  if (permissions.empty()) {
-    ReceivedBadMessage();
+  if (!context_->render_frame_host()) {
     std::move(callback).Run(EmbeddedPermissionControlResult::kNotSupported);
     return;
   }
 
-  if (!std::ranges::all_of(permissions, &ValidatePermissionDescriptor)) {
-    ReceivedBadMessage();
-    std::move(callback).Run(EmbeddedPermissionControlResult::kNotSupported);
-    return;
-  }
-  const base::Feature* required_feature = nullptr;
-  switch (descriptor->detail->which()) {
-    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
-        kGeolocation:
-      required_feature = &blink::features::kGeolocationElement;
-      break;
-    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
-        kInstall:
-      required_feature = &blink::features::kInstallElement;
-      break;
-    case blink::mojom::EmbeddedPermissionControlDescriptorExtension::Tag::
-        kUserMedia:
-      required_feature = &blink::features::kUserMediaElement;
-      break;
-  }
-
-  if (!base::FeatureList::IsEnabled(*required_feature)) {
+  const base::Feature& required_feature =
+      GetRequiredFeatureForPageEmbeddedPermission(descriptor);
+  if (!base::FeatureList::IsEnabled(required_feature)) {
     bad_message::ReceivedBadMessage(
         context_->render_frame_host()->GetProcess(),
         bad_message::PSI_REQUEST_EMBEDDED_PERMISSION_WITHOUT_FEATURE);
@@ -363,14 +351,43 @@
     return;
   }
 
-  if (auto* browser_context = context_->GetBrowserContext()) {
-    if (HasDuplicatesOrInvalidPermissions(permissions) ||
-        !CheckPageEmbeddedPermissionTypes(permissions, descriptor)) {
-      ReceivedBadMessage();
+  std::optional<std::set<PermissionName>> permission_names =
+      GetValidEmbeddedPermissionNames(permissions, descriptor);
+  if (!permission_names) {
+    ReceivedBadMessage();
+    std::move(callback).Run(EmbeddedPermissionControlResult::kNotSupported);
+    return;
+  }
+
+  bool has_user_activation =
+      context_->render_frame_host()->HasTransientUserActivation() ||
+      base::FeatureList::IsEnabled(
+          blink::features::kBypassPepcSecurityForTesting);
+  if (!has_user_activation) {
+    std::move(callback).Run(EmbeddedPermissionControlResult::kNotSupported);
+    return;
+  }
+
+  WebContents* web_contents =
+      WebContents::FromRenderFrameHost(context_->render_frame_host());
+  if (!web_contents) {
+    std::move(callback).Run(EmbeddedPermissionControlResult::kNotSupported);
+    return;
+  }
+
+  if (!base::FeatureList::IsEnabled(
+          blink::features::kBypassPepcSecurityForTesting)) {
+    auto* checker = EmbeddedPermissionControlChecker::GetForPage(
+        web_contents->GetPrimaryPage());
+    if (!checker ||
+        !checker->HasPageEmbeddedPermission(
+            GetCheckerSourceForDescriptor(descriptor), *permission_names)) {
       std::move(callback).Run(EmbeddedPermissionControlResult::kNotSupported);
       return;
     }
+  }
 
+  if (auto* browser_context = context_->GetBrowserContext()) {
     std::vector<PermissionStatus> initial_statuses;
     initial_statuses.reserve(permissions.size());
     for (const auto& permission : permissions) {
@@ -380,8 +397,8 @@
 
     RequestPermissionsInternal(
         browser_context,
-        PermissionRequestDescription(std::move(permissions),
-                                     std::move(descriptor)),
+        PermissionRequestDescription(
+            std::move(permissions), std::move(descriptor), has_user_activation),
         base::BindOnce(&EmbeddedPermissionRequestCallbackWrapper,
                        initial_statuses, std::move(callback)));
   } else {
diff --git a/content/public/browser/permission_request_description.cc b/content/public/browser/permission_request_description.cc
index 9060297..e18c155 100644
--- a/content/public/browser/permission_request_description.cc
+++ b/content/public/browser/permission_request_description.cc
@@ -24,9 +24,10 @@
 
 PermissionRequestDescription::PermissionRequestDescription(
     std::vector<blink::mojom::PermissionDescriptorPtr> permissions,
-    blink::mojom::EmbeddedPermissionRequestDescriptorPtr descriptor)
+    blink::mojom::EmbeddedPermissionRequestDescriptorPtr descriptor,
+    bool user_gesture)
     : permissions(std::move(permissions)),
-      user_gesture(true),
+      user_gesture(user_gesture),
       embedded_permission_request_descriptor(std::move(descriptor)) {
   CHECK(embedded_permission_request_descriptor);
 }
diff --git a/content/public/browser/permission_request_description.h b/content/public/browser/permission_request_description.h
index 878eeae8..9d274bd 100644
--- a/content/public/browser/permission_request_description.h
+++ b/content/public/browser/permission_request_description.h
@@ -29,9 +29,10 @@
       bool user_gesture = false,
       const GURL& requesting_origin = GURL());
 
-  explicit PermissionRequestDescription(
+  PermissionRequestDescription(
       std::vector<blink::mojom::PermissionDescriptorPtr> permissions,
-      blink::mojom::EmbeddedPermissionRequestDescriptorPtr descriptor);
+      blink::mojom::EmbeddedPermissionRequestDescriptorPtr descriptor,
+      bool user_gesture);
 
   PermissionRequestDescription& operator=(const PermissionRequestDescription&) =
       delete;
diff --git a/components/permissions/pepc_initiated_permission_request_unittest.cc b/components/permissions/pepc_initiated_permission_request_unittest.cc
index cf24551..4da4d9cf 100644
--- a/components/permissions/pepc_initiated_permission_request_unittest.cc
+++ b/components/permissions/pepc_initiated_permission_request_unittest.cc
@@ -58,7 +58,8 @@
     scoped_feature_list_.InitWithFeatures(
         /* enabled_features */ {blink::features::kUserMediaElement,
                                 blink::features::kUserMediaElementLegacy,
-                                blink::features::kGeolocationElement},
+                                blink::features::kGeolocationElement,
+                                blink::features::kBypassPepcSecurityForTesting},
         /* disabled_features */ {});
   }
   PEPCInitiatedPermissionRequestTest(
diff --git a/content/browser/permissions/embedded_permission_control_checker_unittest.cc b/content/browser/permissions/embedded_permission_control_checker_unittest.cc
index 173dfe9..dffd86c4 100644
--- a/content/browser/permissions/embedded_permission_control_checker_unittest.cc
+++ b/content/browser/permissions/embedded_permission_control_checker_unittest.cc
@@ -247,6 +247,73 @@
   pending_client_2->ExpectEmbeddedPermissionControlRegistered();
 }
 
+TEST_F(EmbeddedPermissionControlCheckerTest, HasPageEmbeddedPermission) {
+  auto* checker = EmbeddedPermissionControlChecker::GetOrCreateForPage(
... (file diff truncated)

diff --git a/components/permissions/pepc_initiated_permission_request_unittest.cc b/components/permissions/pepc_initiated_permission_request_unittest.cc
index cf24551..4da4d9cf 100644
--- a/components/permissions/pepc_initiated_permission_request_unittest.cc
+++ b/components/permissions/pepc_initiated_permission_request_unittest.cc
@@ -58,7 +58,8 @@
     scoped_feature_list_.InitWithFeatures(
         /* enabled_features */ {blink::features::kUserMediaElement,
                                 blink::features::kUserMediaElementLegacy,
-                                blink::features::kGeolocationElement},
+                                blink::features::kGeolocationElement,
+                                blink::features::kBypassPepcSecurityForTesting},
         /* disabled_features */ {});
   }
   PEPCInitiatedPermissionRequestTest(
diff --git a/content/browser/permissions/embedded_permission_control_checker_unittest.cc b/content/browser/permissions/embedded_permission_control_checker_unittest.cc
index 173dfe9..dffd86c4 100644
--- a/content/browser/permissions/embedded_permission_control_checker_unittest.cc
+++ b/content/browser/permissions/embedded_permission_control_checker_unittest.cc
@@ -247,6 +247,73 @@
   pending_client_2->ExpectEmbeddedPermissionControlRegistered();
 }
 
+TEST_F(EmbeddedPermissionControlCheckerTest, HasPageEmbeddedPermission) {
+  auto* checker = EmbeddedPermissionControlChecker::GetOrCreateForPage(
+      web_contents()->GetPrimaryPage());
+  ASSERT_TRUE(checker);
+
+  EXPECT_FALSE(checker->HasPageEmbeddedPermission(
+      EmbeddedPermissionControlChecker::Source::kUserMediaElement,
+      {PermissionName::AUDIO_CAPTURE}));
+
+  auto client = CreateEmbeddedPermissionControlClient(
+      {PermissionName::AUDIO_CAPTURE}, CapabilityElementSource::kUserMedia);
+  client->ExpectEmbeddedPermissionControlRegistered();
+
+  EXPECT_TRUE(checker->HasPageEmbeddedPermission(
+      EmbeddedPermissionControlChecker::Source::kUserMediaElement,
+      {PermissionName::AUDIO_CAPTURE}));
+  EXPECT_FALSE(checker->HasPageEmbeddedPermission(
+      EmbeddedPermissionControlChecker::Source::kUserMediaElement,
+      {PermissionName::VIDEO_CAPTURE}));
+  EXPECT_FALSE(checker->HasPageEmbeddedPermission(
+      EmbeddedPermissionControlChecker::Source::kGeolocationElement,
+      {PermissionName::AUDIO_CAPTURE}));
+
+  auto client_2 = CreateEmbeddedPermissionControlClient(
+      {PermissionName::AUDIO_CAPTURE}, CapabilityElementSource::kUserMedia);
+  client_2->ExpectEmbeddedPermissionControlRegistered();
+
+  EXPECT_TRUE(checker->HasPageEmbeddedPermission(
+      EmbeddedPermissionControlChecker::Source::kUserMediaElement,
+      {PermissionName::AUDIO_CAPTURE}));
+
+  client.reset();
+  base::RunLoop().RunUntilIdle();
+
+  // Still true because client_2 is registered.
+  EXPECT_TRUE(checker->HasPageEmbeddedPermission(
+      EmbeddedPermissionControlChecker::Source::kUserMediaElement,
+      {PermissionName::AUDIO_CAPTURE}));
+
+  client_2.reset();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_FALSE(checker->HasPageEmbeddedPermission(
+      EmbeddedPermissionControlChecker::Source::kUserMediaElement,
+      {PermissionName::AUDIO_CAPTURE}));
+
+  // Test combined permissions.
+  auto combined_client = CreateEmbeddedPermissionControlClient(
+      {PermissionName::AUDIO_CAPTURE, PermissionName::VIDEO_CAPTURE},
+      CapabilityElementSource::kUserMedia);
+  combined_client->ExpectEmbeddedPermissionControlRegistered();
+
+  EXPECT_TRUE(checker->HasPageEmbeddedPermission(
+      EmbeddedPermissionControlChecker::Source::kUserMediaElement,
+      {PermissionName::AUDIO_CAPTURE, PermissionName::VIDEO_CAPTURE}));
+  EXPECT_FALSE(checker->HasPageEmbeddedPermission(
+      EmbeddedPermissionControlChecker::Source::kUserMediaElement,
+      {PermissionName::AUDIO_CAPTURE}));
+
+  combined_client.reset();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_FALSE(checker->HasPageEmbeddedPermission(
+      EmbeddedPermissionControlChecker::Source::kUserMediaElement,
+      {PermissionName::AUDIO_CAPTURE, PermissionName::VIDEO_CAPTURE}));
+}
+
 class GeolocationEmbeddedPermissionControlCheckerTest
     : public EmbeddedPermissionControlCheckerTest {
  public: