From 9a6fcf26c50b7aa77f158bb1a807b0d0137ce624 Mon Sep 17 00:00:00 2001
From: Danil Somsikov <dsv@chromium.org>
Date: Thu, 26 Feb 2026 00:52:26 -0800
Subject: [PATCH] Refactor DevTools cookie handling for better access control.

This change introduces a static `NetworkHandler::CanAccessCookie` method to centralize permission checks for accessing cookies. The `NetworkHandler::ClearCookies` static method is added to provide a unified way to clear cookies, used by both `Network.clearBrowserCookies` and `Storage.clearCookies`. The `SetCookies` methods are updated to use the new permission check. The `FilterCookies` helper is made more flexible by using optional parameters and a flag for partition key filtering.

Bug: 479673903
Change-Id: Idcff402bff288c58771d42b75f143b83557279e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7597631
Commit-Queue: Danil Somsikov <dsv@chromium.org>
Auto-Submit: Danil Somsikov <dsv@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1590697}
---

diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index 9e29f4d0..ac7cb63d 100644
--- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -2764,6 +2764,76 @@
   EXPECT_EQ(2u, found);
 }
 
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, CookiePermissions) {
+  SetNotAttachableHosts({"b.test"});
+  content::SetupCrossSiteRedirector(embedded_test_server());
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::string cookies_to_set = "/set-cookie?foo=bar";
+  GURL url = embedded_test_server()->GetURL("b.test", cookies_to_set);
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  url = embedded_test_server()->GetURL("a.test", cookies_to_set);
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  Attach();
+
+  // Try to set a cookie on b.test via protocol.
+  base::DictValue set_params;
+  set_params.Set("name", "proto_cookie");
+  set_params.Set("value", "proto_val");
+  set_params.Set("domain", "b.test");
+  set_params.Set("path", "/");
+  SendCommandSync("Network.setCookie", std::move(set_params));
+  EXPECT_THAT(
+      error()->FindInt("code"),
+      testing::Optional(static_cast<int>(crdtp::DispatchCode::SERVER_ERROR)));
+  EXPECT_EQ(*error()->FindString("message"), "Permission denied");
+
+  // Try to set cookies on b.test via protocol.
+  base::DictValue set_cookies_params;
+  base::ListValue cookies_list;
+  base::DictValue cookie;
+  cookie.Set("name", "proto_cookie_2");
+  cookie.Set("value", "val");
+  cookie.Set("domain", "b.test");
+  cookie.Set("path", "/");
+  cookies_list.Append(std::move(cookie));
+  set_cookies_params.Set("cookies", std::move(cookies_list));
+  SendCommandSync("Network.setCookies", std::move(set_cookies_params));
+  EXPECT_THAT(
+      error()->FindInt("code"),
+      testing::Optional(static_cast<int>(crdtp::DispatchCode::INVALID_PARAMS)));
+  EXPECT_EQ(*error()->FindString("message"), "Invalid cookie fields");
+
+  // Try to delete cookie on b.test via protocol.
+  base::DictValue del_params;
+  del_params.Set("name", "foo");
+  del_params.Set("domain", "b.test");
+  SendCommandSync("Network.deleteCookies", std::move(del_params));
+  EXPECT_FALSE(error());
+
+  // Try to clear browser cookies.
+  SendCommandSync("Network.clearBrowserCookies");
+  EXPECT_FALSE(error());
+
+  // Verify a.test cookie is gone.
+  const base::ListValue* cookies =
+      SendCommandSync("Network.getAllCookies")->FindList("cookies");
+  ASSERT_TRUE(cookies);
+  EXPECT_EQ(0u, cookies->size());
+
+  Detach();
+
+  // Verify b.test cookie is still there.
+  GURL url_b_echo =
+      embedded_test_server()->GetURL("b.test", "/echoheader?Cookie");
+  EXPECT_TRUE(NavigateToURL(shell(), url_b_echo));
+  std::string content =
+      EvalJs(shell()->web_contents(), "document.body.innerText")
+          .ExtractString();
+  EXPECT_THAT(content, testing::HasSubstr("foo=bar"));
+}
+
 IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
                        ReturnsCookiesOnlyForAttachableUrls) {
   SetNotAttachableHosts({"b.test"});
diff --git a/content/browser/devtools/protocol/network_handler.cc b/content/browser/devtools/protocol/network_handler.cc
index ca70338f..673fdb06 100644
--- a/content/browser/devtools/protocol/network_handler.cc
+++ b/content/browser/devtools/protocol/network_handler.cc
@@ -15,6 +15,7 @@
 
 #include "base/barrier_closure.h"
 #include "base/base64.h"
+#include "base/check_deref.h"
 #include "base/command_line.h"
 #include "base/containers/queue.h"
 #include "base/containers/span.h"
@@ -312,39 +313,49 @@
 namespace {
 std::vector<net::CanonicalCookie> FilterCookies(
     const std::vector<net::CanonicalCookie>& cookies,
-    const std::string& name,
-    const std::string& normalized_domain,
-    const std::string& path,
-    std::unique_ptr<Network::CookiePartitionKey> partition_key) {
+    const std::optional<std::string>& name,
+    const std::optional<std::string>& normalized_domain,
+    const std::optional<std::string>& path,
+    const Network::CookiePartitionKey* partition_key,
+    bool filter_by_partition_key) {
   std::vector<net::CanonicalCookie> result;
 
   for (const auto& cookie : cookies) {
-    if (cookie.Name() != name)
+    if (name.has_value() && cookie.Name() != name.value()) {
       continue;
-    if (cookie.Domain() != normalized_domain)
+    }
+    if (normalized_domain.has_value() &&
+        cookie.Domain() != normalized_domain.value()) {
       continue;
-    if (!path.empty() && cookie.Path() != path)
-      continue;
-
-    if (!!cookie.PartitionKey() != !!partition_key) {
+    }
+    if (path.has_value() && !path.value().empty() &&
+        cookie.Path() != path.value()) {
       continue;
     }
 
-    if (cookie.PartitionKey().has_value()) {
-      base::expected<net::CookiePartitionKey::SerializedCookiePartitionKey,
-                     std::string>
-          serialized_result =
-              net::CookiePartitionKey::Serialize(cookie.PartitionKey());
-
-      if (!serialized_result.has_value() ||
-          (serialized_result->TopLevelSite() !=
-           partition_key->GetTopLevelSite())) {
+    if (filter_by_partition_key) {
+      if (!!cookie.PartitionKey() != !!partition_key) {
         continue;
       }
 
-      if (serialized_result->has_cross_site_ancestor() !=
-           partition_key->GetHasCrossSiteAncestor()) {
-        continue;
+      if (cookie.PartitionKey().has_value()) {
+        base::expected<net::CookiePartitionKey::SerializedCookiePartitionKey,
+                       std::string>
+            serialized_result =
+                net::CookiePartitionKey::Serialize(cookie.PartitionKey());
+
+        if (!serialized_result.has_value() ||
+            (serialized_result->TopLevelSite() !=
+             const_cast<Network::CookiePartitionKey*>(partition_key)
+                 ->GetTopLevelSite())) {
+          continue;
+        }
+
+        if (serialized_result->has_cross_site_ancestor() !=
+            const_cast<Network::CookiePartitionKey*>(partition_key)
+                ->GetHasCrossSiteAncestor()) {
+          continue;
+        }
       }
     }
 
@@ -354,20 +365,26 @@
   return result;
 }
 
-void DeleteFilteredCookies(
-    network::mojom::CookieManager* cookie_manager,
-    const std::string& name,
-    const std::string& normalized_domain,
-    const std::string& path,
-    std::unique_ptr<Network::CookiePartitionKey> partition_key,
-    std::unique_ptr<DeleteCookiesCallback> callback,
-    const std::vector<net::CanonicalCookie>& cookies) {
-  std::vector<net::CanonicalCookie> filtered_list = FilterCookies(
-      cookies, name, normalized_domain, path, std::move(partition_key));
+void DeleteFilteredCookies(network::mojom::CookieManager* cookie_manager,
+                           const std::optional<std::string>& name,
+                           const std::optional<std::string>& normalized_domain,
+                           const std::optional<std::string>& path,
+                           const Network::CookiePartitionKey* partition_key,

diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index 9e29f4d0..ac7cb63d 100644
--- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -2764,6 +2764,76 @@
   EXPECT_EQ(2u, found);
 }
 
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, CookiePermissions) {
+  SetNotAttachableHosts({"b.test"});
+  content::SetupCrossSiteRedirector(embedded_test_server());
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  std::string cookies_to_set = "/set-cookie?foo=bar";
+  GURL url = embedded_test_server()->GetURL("b.test", cookies_to_set);
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  url = embedded_test_server()->GetURL("a.test", cookies_to_set);
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  Attach();
+
+  // Try to set a cookie on b.test via protocol.
+  base::DictValue set_params;
+  set_params.Set("name", "proto_cookie");
+  set_params.Set("value", "proto_val");
+  set_params.Set("domain", "b.test");
+  set_params.Set("path", "/");
+  SendCommandSync("Network.setCookie", std::move(set_params));
+  EXPECT_THAT(
+      error()->FindInt("code"),
+      testing::Optional(static_cast<int>(crdtp::DispatchCode::SERVER_ERROR)));
+  EXPECT_EQ(*error()->FindString("message"), "Permission denied");
+
+  // Try to set cookies on b.test via protocol.
+  base::DictValue set_cookies_params;
+  base::ListValue cookies_list;
+  base::DictValue cookie;
+  cookie.Set("name", "proto_cookie_2");
+  cookie.Set("value", "val");
+  cookie.Set("domain", "b.test");
+  cookie.Set("path", "/");
+  cookies_list.Append(std::move(cookie));
+  set_cookies_params.Set("cookies", std::move(cookies_list));
+  SendCommandSync("Network.setCookies", std::move(set_cookies_params));
+  EXPECT_THAT(
+      error()->FindInt("code"),
+      testing::Optional(static_cast<int>(crdtp::DispatchCode::INVALID_PARAMS)));
+  EXPECT_EQ(*error()->FindString("message"), "Invalid cookie fields");
+
+  // Try to delete cookie on b.test via protocol.
+  base::DictValue del_params;
+  del_params.Set("name", "foo");
+  del_params.Set("domain", "b.test");
+  SendCommandSync("Network.deleteCookies", std::move(del_params));
+  EXPECT_FALSE(error());
+
+  // Try to clear browser cookies.
+  SendCommandSync("Network.clearBrowserCookies");
+  EXPECT_FALSE(error());
+
+  // Verify a.test cookie is gone.
+  const base::ListValue* cookies =
+      SendCommandSync("Network.getAllCookies")->FindList("cookies");
+  ASSERT_TRUE(cookies);
+  EXPECT_EQ(0u, cookies->size());
+
+  Detach();
+
+  // Verify b.test cookie is still there.
+  GURL url_b_echo =
+      embedded_test_server()->GetURL("b.test", "/echoheader?Cookie");
+  EXPECT_TRUE(NavigateToURL(shell(), url_b_echo));
+  std::string content =
+      EvalJs(shell()->web_contents(), "document.body.innerText")
+          .ExtractString();
+  EXPECT_THAT(content, testing::HasSubstr("foo=bar"));
+}
+
 IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
                        ReturnsCookiesOnlyForAttachableUrls) {
   SetNotAttachableHosts({"b.test"});
diff --git a/third_party/blink/web_tests/http/tests/inspector-protocol/storage/cookies-expected.txt b/third_party/blink/web_tests/http/tests/inspector-protocol/storage/cookies-expected.txt
index 30ccb31..21690bf 100644
--- a/third_party/blink/web_tests/http/tests/inspector-protocol/storage/cookies-expected.txt
+++ b/third_party/blink/web_tests/http/tests/inspector-protocol/storage/cookies-expected.txt
@@ -22,7 +22,7 @@
 {
     error : {
         code : -32602
-        message : Failed to find browser context for id invalid
+        message : browserContextId is only allowed for Browser target
     }
     id : <number>
     sessionId : <string>