From de2829e40077d8819ef7e7814a52f339630b0ca3 Mon Sep 17 00:00:00 2001
From: Danil Somsikov <dsv@chromium.org>
Date: Mon, 04 May 2026 09:51:35 -0700
Subject: [PATCH] Verify sessionId in incoming protocol messages from renderer

In DevTools "flattened" protocol mode, the renderer process is responsible
for including the correct sessionId in every protocol response and
notification. Previously, the browser process forwarded these messages to
the client without verification. A compromised renderer could deliberately
omit or spoof the sessionId, potentially allowing it to inject events into
the root session or other child sessions.

This CL fixes the vulnerability by having the browser process verify the
sessionId of every protocol message originating from a renderer-side 
session. If the sessionId is missing, incorrect, or unexpectedly present 
(in the case of the root session), the browser now considers the renderer
compromised and terminates it using bad_message::ReceivedBadMessage.

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

diff --git a/content/browser/devtools/devtools_agent_host_impl.cc b/content/browser/devtools/devtools_agent_host_impl.cc
index e05cca9..ad27a98 100644
--- a/content/browser/devtools/devtools_agent_host_impl.cc
+++ b/content/browser/devtools/devtools_agent_host_impl.cc
@@ -315,6 +315,23 @@
   return it == session_by_client_.end() ? nullptr : it->second.get();
 }
 
+DevToolsSession* DevToolsAgentHostImpl::GetSessionByIdForTesting(
+    const std::string& session_id) {
+  DevToolsSession* session = nullptr;
+  if (session_id.empty()) {
+    session = sessions_.empty() ? nullptr : sessions_.front();
+  } else {
+    for (DevToolsSession* root_session : sessions_) {
+      if (root_session->HasChildSession(session_id)) {
+        session = root_session->GetSessionById(session_id);
+        break;
+      }
+    }
+  }
+  CHECK(session) << "Session not found: " << session_id;
+  return session;
+}
+
 bool DevToolsAgentHostImpl::AttachInternal(
     std::unique_ptr<DevToolsSession> session_owned) {
   scoped_refptr<DevToolsAgentHostImpl> protect(this);
diff --git a/content/browser/devtools/devtools_agent_host_impl.h b/content/browser/devtools/devtools_agent_host_impl.h
index 4a83684a..78239cec 100644
--- a/content/browser/devtools/devtools_agent_host_impl.h
+++ b/content/browser/devtools/devtools_agent_host_impl.h
@@ -121,6 +121,8 @@
 
   base::ProcessId GetProcessId() const { return process_id_; }
 
+  DevToolsSession* GetSessionByIdForTesting(const std::string& session_id);
+
  protected:
   explicit DevToolsAgentHostImpl(const std::string& id);
   ~DevToolsAgentHostImpl() override;
diff --git a/content/browser/devtools/devtools_session.cc b/content/browser/devtools/devtools_session.cc
index b473017..213146b 100644
--- a/content/browser/devtools/devtools_session.cc
+++ b/content/browser/devtools/devtools_session.cc
@@ -10,7 +10,9 @@
 #include "base/containers/flat_set.h"
 #include "base/functional/bind.h"
 #include "base/trace_event/trace_event.h"
+#include "content/browser/bad_message.h"
 #include "content/browser/devtools/devtools_manager.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/browser/devtools/protocol/devtools_domain_handler.h"
 #include "content/browser/devtools/protocol/protocol.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
@@ -537,10 +539,19 @@
 // parsed and sent as is, since a renderer may be compromised; so therefore,
 // we're not sending them via the DevToolsAgentHostClientChannel interface
 // (::DispatchProtocolMessageToClient) but directly to the client instead.
-static void DispatchProtocolResponseOrNotification(
+void DevToolsSession::DispatchProtocolResponseOrNotification(
     DevToolsAgentHostClient* client,
     DevToolsAgentHostImpl* agent_host,
-    blink::mojom::DevToolsMessagePtr message) {
+    blink::mojom::DevToolsMessagePtr message,
+    const std::string& session_id) {
+  base::span<const uint8_t> message_span = message->data;
+  if (!ValidateSessionId(session_id, message_span)) {
+    if (RenderProcessHost* process_host = agent_host->GetProcessHost()) {
+      bad_message::ReceivedBadMessage(
+          process_host, bad_message::RFH_INCONSISTENT_DEVTOOLS_MESSAGE);
+    }
+    return;
+  }
   client->DispatchProtocolMessage(agent_host, message->data);
 }
 
@@ -560,7 +571,7 @@
   pending_messages_.erase(it->second);
   waiting_for_response_.erase(it);
   DispatchProtocolResponseOrNotification(client_, agent_host_,
-                                         std::move(message));
+                                         std::move(message), session_id_);
   // |this| may be deleted at this point.
 }
 
@@ -569,7 +580,7 @@
     blink::mojom::DevToolsSessionStatePtr updates) {
   ApplySessionStateUpdates(std::move(updates));
   DispatchProtocolResponseOrNotification(client_, agent_host_,
-                                         std::move(message));
+                                         std::move(message), session_id_);
   // |this| may be deleted at this point.
 }
 
@@ -694,4 +705,54 @@
              : nullptr;
 }
 
+DevToolsSession* DevToolsSession::GetSessionById(const std::string& session_id) {
+  auto it = child_sessions_.find(session_id);
+  return it == child_sessions_.end() ? nullptr : it->second.get();
+}
+
+// static
+bool DevToolsSession::ValidateSessionId(const std::string& expected_session_id,
+                                        base::span<const uint8_t> message) {
+  std::vector<uint8_t> cbor_message;
+  crdtp::span<uint8_t> span_message = crdtp::SpanFrom(message);
+
+  if (!crdtp::cbor::IsCBORMessage(span_message)) {
+    if (!crdtp::json::ConvertJSONToCBOR(span_message, &cbor_message).ok()) {
+      return false;  // Safely terminate renderer on malformed JSON
+    }
+    span_message = crdtp::SpanFrom(cbor_message);
+  }
+
+  // Do NOT use crdtp::Dispatchable here. It enforces the presence of both
+  // 'id' and 'method', which are not guaranteed in Responses and Notifications.
+  crdtp::span<uint8_t> extracted_session_id =
+      crdtp::cbor::GetString8ValueFromMap(span_message,
+                                          crdtp::SpanFrom("sessionId"));
+
+  if (expected_session_id.empty()) {
+    if (!extracted_session_id.empty()) {
+      DLOG(ERROR) << "Root session expected no sessionId but received one: "
+                  << (extracted_session_id.empty()
+                          ? ""
+                          : std::string(extracted_session_id.begin(),
+                                        extracted_session_id.end()));
+      return false;
+    }
+    return true;
+  } else {
+    if (extracted_session_id.empty() ||
+        !crdtp::SpanEquals(crdtp::SpanFrom(expected_session_id),
+                           extracted_session_id)) {
+      DLOG(ERROR) << "Child session expected sessionId: " << expected_session_id
+                  << ", but got: "
+                  << (extracted_session_id.empty()
+                          ? ""
+                          : std::string(extracted_session_id.begin(),
+                                        extracted_session_id.end()));
+      return false;
+    }
+    return true;
+  }
+}
+
 }  // namespace content
diff --git a/content/browser/devtools/devtools_session.h b/content/browser/devtools/devtools_session.h
index 036ca007..3f5efa1 100644
--- a/content/browser/devtools/devtools_session.h
+++ b/content/browser/devtools/devtools_session.h
@@ -12,6 +12,7 @@
 
 #include "base/containers/flat_map.h"
 #include "base/containers/span.h"
+#include "base/functional/callback_forward.h"
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
@@ -29,6 +30,8 @@
 class DevToolsAgentHostClient;
 class DevToolsAgentHostImpl;
 class DevToolsExternalAgentProxyDelegate;
+class DevToolsSession;
+class RenderProcessHost;
 
 namespace protocol {
 class DevToolsDomainHandler;
@@ -127,11 +130,16 @@
                                       base::OnceClosure resume_callback);

diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index 3b80e4c..d912305 100644
--- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -13,11 +13,13 @@
 #include "base/base64.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/containers/span.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
 #include "base/json/json_reader.h"
+#include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/memory/raw_ptr.h"
 #include "base/strings/safe_sprintf.h"
@@ -35,6 +37,8 @@
 #include "components/download/public/common/download_file_impl.h"
 #include "components/download/public/common/download_task_runner.h"
 #include "components/services/storage/shared_storage/shared_storage_manager.h"
+#include "content/browser/devtools/devtools_agent_host_impl.h"
+#include "content/browser/devtools/devtools_session.h"
 #include "content/browser/devtools/protocol/browser_handler.h"
 #include "content/browser/devtools/protocol/devtools_download_manager_delegate.h"
 #include "content/browser/devtools/protocol/devtools_protocol_test_support.h"
@@ -44,6 +48,7 @@
 #include "content/browser/host_zoom_map_impl.h"
 #include "content/browser/preloading/prerender/prerender_final_status.h"
 #include "content/browser/renderer_host/navigator.h"
+#include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_view_base.h"
 #include "content/browser/screen_orientation/screen_orientation_provider.h"
 #include "content/browser/service_worker/embedded_worker_test_helper.h"
@@ -55,6 +60,7 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/ssl_status.h"
@@ -94,6 +100,9 @@
 #include "third_party/blink/public/common/page/page_zoom.h"
 #include "third_party/boringssl/src/include/openssl/nid.h"
 #include "third_party/boringssl/src/include/openssl/ssl.h"
+#include "third_party/inspector_protocol/crdtp/cbor.h"
+#include "third_party/inspector_protocol/crdtp/dispatch.h"
+#include "third_party/inspector_protocol/crdtp/json.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "third_party/zlib/google/compression_utils.h"
@@ -1557,6 +1566,188 @@
   EXPECT_EQ(frame_target_id, *params.FindString("targetId"));
 }
 
+class FlattenedDevToolsProtocolTest
+    : public SitePerProcessDevToolsProtocolTest {
+ public:
+  void DispatchProtocolNotification(DevToolsAgentHostImpl* host,
+                                    const std::string& session_id,
+                                    const std::string& message_json,
+                                    bool expect_crash,
+                                    bool use_cbor = false) {
+    DevToolsSession* session = host->GetSessionByIdForTesting(session_id);
+
+    std::unique_ptr<RenderProcessHostWatcher> watcher;
+    if (expect_crash) {
+      watcher = std::make_unique<RenderProcessHostWatcher>(
+          session->GetAgentHost()->GetProcessHost(),
+          RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+    }
+
+    blink::mojom::DevToolsMessagePtr message =
+        blink::mojom::DevToolsMessage::New();
+    if (use_cbor) {
+      std::vector<uint8_t> cbor;
+      crdtp::Status status =
+          crdtp::json::ConvertJSONToCBOR(crdtp::SpanFrom(message_json), &cbor);
+      CHECK(status.ok()) << status.ToASCIIString();
+      message->data = mojo_base::BigBuffer(cbor);
+    } else {
+      message->data = mojo_base::BigBuffer(base::as_byte_span(message_json));
+    }
+
+    session->DispatchProtocolNotification(std::move(message), nullptr);
+
+    if (expect_crash) {
+      watcher->Wait();
+      EXPECT_FALSE(watcher->did_exit_normally());
+    }
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(FlattenedDevToolsProtocolTest,
+                       SessionIdValidationSucceeds) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL test_url =
+      embedded_test_server()->GetURL("/devtools/page-with-oopif.html");
+  NavigateToURLBlockUntilNavigationsComplete(shell(), test_url, 1);
+  Attach();
+
+  // Root session of the client.
+  DevToolsAgentHostImpl* host_impl =
+      static_cast<DevToolsAgentHostImpl*>(agent_host_.get());
+
+  // Enable auto-attach to attach OOPIF subframe.
+  base::DictValue command_params;
+  command_params.Set("autoAttach", true);
+  command_params.Set("waitForDebuggerOnStart", false);
+  command_params.Set("flatten", true);
+  SendCommandSync("Target.setAutoAttach", std::move(command_params));
+
+  // Get session id of subframe.
+  auto notification = WaitForNotification("Target.attachedToTarget", true);
+  const std::string* session_id_ptr = notification.FindString("sessionId");
+  ASSERT_TRUE(session_id_ptr);
+  std::string session_id = *session_id_ptr;
+
+  // Simulate a message from the renderer that HAS the correct sessionId.
+  std::string message_json =
+      "{\"method\":\"Test.test\",\"params\":{},\"sessionId\":\"" + session_id +
+      "\"}";
+
+  ClearNotifications();
+  DispatchProtocolNotification(host_impl, session_id, message_json, false);
+
+  WaitForNotification("Test.test", true);
+}
+
+IN_PROC_BROWSER_TEST_F(FlattenedDevToolsProtocolTest,
+                       SessionIdValidationFails) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL test_url =
+      embedded_test_server()->GetURL("/devtools/page-with-oopif.html");
+  NavigateToURLBlockUntilNavigationsComplete(shell(), test_url, 1);
+  Attach();
+
+  // Root session, auto-attach OOPIF.
+  base::DictValue command_params;
+  command_params.Set("autoAttach", true);
+  command_params.Set("waitForDebuggerOnStart", false);
+  command_params.Set("flatten", true);
+  SendCommandSync("Target.setAutoAttach", std::move(command_params));
+
+  auto notification = WaitForNotification("Target.attachedToTarget", true);
+  const std::string* session_id_ptr = notification.FindString("sessionId");
+  ASSERT_TRUE(session_id_ptr);
+  std::string session_id = *session_id_ptr;
+
+  DevToolsAgentHostImpl* host_impl =
+      static_cast<DevToolsAgentHostImpl*>(agent_host_.get());
+
+  // Simulate a message from the renderer that MISSES the sessionId.
+  std::string message_json = "{\"method\":\"Test.test\",\"params\":{}}";
+
+  DispatchProtocolNotification(host_impl, session_id, message_json, true);
+}
+
+IN_PROC_BROWSER_TEST_F(FlattenedDevToolsProtocolTest,
+                       SessionIdValidationFailsWrongId) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL test_url =
+      embedded_test_server()->GetURL("/devtools/page-with-oopif.html");
+  NavigateToURLBlockUntilNavigationsComplete(shell(), test_url, 1);
+  Attach();
+
+  // Root session, auto-attach OOPIF.
+  base::DictValue command_params;
+  command_params.Set("autoAttach", true);
+  command_params.Set("waitForDebuggerOnStart", false);
+  command_params.Set("flatten", true);
+  SendCommandSync("Target.setAutoAttach", std::move(command_params));
+
+  auto notification = WaitForNotification("Target.attachedToTarget", true);
+  const std::string* session_id_ptr = notification.FindString("sessionId");
+  ASSERT_TRUE(session_id_ptr);
+  std::string session_id = *session_id_ptr;
+
+  DevToolsAgentHostImpl* host_impl =
+      static_cast<DevToolsAgentHostImpl*>(agent_host_.get());
+
+  // Simulate a message from the renderer that has a WRONG sessionId.
+  std::string message_json =
+      "{\"method\":\"Test.test\",\"params\":{},\"sessionId\":\"wrong_id\"}";
+
+  DispatchProtocolNotification(host_impl, session_id, message_json, true);
+}
+
+IN_PROC_BROWSER_TEST_F(FlattenedDevToolsProtocolTest,
+                       RootSessionValidationFails) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL test_url =
+      embedded_test_server()->GetURL("/devtools/page-with-oopif.html");
+  NavigateToURLBlockUntilNavigationsComplete(shell(), test_url, 1);
+  Attach();
+
+  DevToolsAgentHostImpl* host_impl =
+      static_cast<DevToolsAgentHostImpl*>(agent_host_.get());
+
+  // Root session message with a sessionId should fail.
+  std::string message_json =
+      "{\"method\":\"Test.test\",\"params\":{},\"sessionId\":\"12345\"}";
+
+  DispatchProtocolNotification(host_impl, "", message_json, true);
+}
+
+IN_PROC_BROWSER_TEST_F(FlattenedDevToolsProtocolTest,
+                       SessionIdValidationFailsCBOR) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL test_url =
+      embedded_test_server()->GetURL("/devtools/page-with-oopif.html");
+  NavigateToURLBlockUntilNavigationsComplete(shell(), test_url, 1);
+  Attach();
+
+  // Root session, auto-attach OOPIF.
+  base::DictValue command_params;
+  command_params.Set("autoAttach", true);
+  command_params.Set("waitForDebuggerOnStart", false);
+  command_params.Set("flatten", true);
+  SendCommandSync("Target.setAutoAttach", std::move(command_params));
+
+  auto notification = WaitForNotification("Target.attachedToTarget", true);
+  const std::string* session_id_ptr = notification.FindString("sessionId");
+  ASSERT_TRUE(session_id_ptr);
+  std::string session_id = *session_id_ptr;
+
+  DevToolsAgentHostImpl* host_impl =
+      static_cast<DevToolsAgentHostImpl*>(agent_host_.get());
+
+  // Simulate a message from the renderer that has a WRONG sessionId in CBOR.
+  std::string message_json =
+      "{\"method\":\"Test.test\",\"params\":{},\"sessionId\":\"wrong_id\"}";
+
+  DispatchProtocolNotification(host_impl, session_id, message_json, true,
+                               /*use_cbor=*/true);
+}
+
 // TODO(crbug.com/440535492): Flaky on Win dbg. Re-enable this test.
 #if BUILDFLAG(IS_WIN) && !defined(NDEBUG)
 #define MAYBE_PageCrashClearsPendingCommands \
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index 6daf10c..a5f74ee 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -676,6 +676,7 @@
     "//testing/gmock",
     "//testing/gtest",
     "//third_party/blink/public/strings:strings_grit",
+    "//third_party/inspector_protocol:crdtp",
     "//third_party/webrtc_overrides:webrtc_component",
     "//third_party/zlib/google:compression_utils",
     "//tools/v8_context_snapshot:buildflags",
@@ -2043,6 +2044,7 @@
     "//testing/gtest",
     "//third_party/angle:includes",
     "//third_party/blink/public:blink",
+    "//third_party/inspector_protocol:crdtp",
     "//third_party/leveldatabase",
     "//third_party/re2",
     "//third_party/zlib",