From 4e48524dc4cb6772193140e0436738de9b28fba2 Mon Sep 17 00:00:00 2001
From: Andrew Paseltiner <apaseltiner@chromium.org>
Date: Thu, 16 Apr 2026 13:16:30 -0700
Subject: [PATCH] Fix logic error and potential memory corruption in NetworkResourcesData.

In NetworkResourcesData::Clear, preserved resources were not being added
back to the request_ids_deque_, leading to a desynchronization between
content_size_ and the deque. This could cause EnsureFreeSpace to pop
from an empty deque, resulting in an out-of-bounds read and potential
memory corruption in Release builds.

This CL:

1. Ensures preserved resource IDs are pushed back into the deque in
   Clear().
2. Adds an explicit empty() check in EnsureFreeSpace() as a
   defense-in-depth measure.
3. Adds CORE_EXPORT to NetworkResourcesData and related classes to
   enable unit testing.
4. Introduces new unit tests to verify the fix and prevent regressions,
   including coverage for resource eviction and size limit updates.

Fixed: 501878477
Change-Id: Ia55d7b64abff0a5dd220481456e515a7d42eac0b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7759758
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1616084}
---

diff --git a/third_party/blink/renderer/core/inspector/build.gni b/third_party/blink/renderer/core/inspector/build.gni
index 9e27b01..b2d7c89e 100644
--- a/third_party/blink/renderer/core/inspector/build.gni
+++ b/third_party/blink/renderer/core/inspector/build.gni
@@ -163,6 +163,7 @@
   "inspector_style_resolver_test.cc",
   "invalidation_set_to_selector_map_test.cc",
   "main_thread_debugger_test.cc",
+  "network_resources_data_test.cc",
   "protocol_parser_test.cc",
   "protocol_unittest.cc",
 ]
diff --git a/third_party/blink/renderer/core/inspector/network_resources_data.cc b/third_party/blink/renderer/core/inspector/network_resources_data.cc
index 694d430..7c442b7 100644
--- a/third_party/blink/renderer/core/inspector/network_resources_data.cc
+++ b/third_party/blink/renderer/core/inspector/network_resources_data.cc
@@ -435,6 +435,10 @@
         resource_data->LoaderId() == preserved_loader_id) {
       preserved_map.Set(resource.key, resource.value);
       content_size_ += resource_data->ContentSize();
+      // Ensure preserved resources are also in the deque to maintain
+      // synchronization between content_size_ and the eviction queue.
+      // See crbug.com/501878477.
+      request_ids_deque_.push_back(resource.key);
     }
   }
   request_id_to_resource_data_map_.swap(preserved_map);
@@ -474,6 +478,12 @@
     return false;
 
   while (content_size_ + size > maximum_resources_content_size_) {
+    // Safety check: if the deque is empty but content_size_ still indicates
+    // we need to free space, we must stop to avoid OOB reads.
+    // See crbug.com/501878477.
+    if (request_ids_deque_.empty()) {
+      break;
+    }
     String request_id = request_ids_deque_.TakeFirst();
     ResourceData* resource_data = ResourceDataForRequestId(request_id);
     if (resource_data)
diff --git a/third_party/blink/renderer/core/inspector/network_resources_data.h b/third_party/blink/renderer/core/inspector/network_resources_data.h
index 3d7ccda24..899f7d6 100644
--- a/third_party/blink/renderer/core/inspector/network_resources_data.h
+++ b/third_party/blink/renderer/core/inspector/network_resources_data.h
@@ -30,6 +30,7 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_INSPECTOR_NETWORK_RESOURCES_DATA_H_
 
 #include "net/cert/x509_certificate.h"
+#include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/html/parser/text_resource_decoder.h"
 #include "third_party/blink/renderer/core/inspector/inspector_page_agent.h"
@@ -80,11 +81,11 @@
   bool include_credentials_;
 };
 
-class NetworkResourcesData final
+class CORE_EXPORT NetworkResourcesData final
     : public GarbageCollected<NetworkResourcesData> {
  public:
-  class ResourceData final : public GarbageCollected<ResourceData>,
-                             public FontResourceClearDataObserver {
+  class CORE_EXPORT ResourceData final : public GarbageCollected<ResourceData>,
+                                         public FontResourceClearDataObserver {
     friend class NetworkResourcesData;
 
    public:
diff --git a/third_party/blink/renderer/core/inspector/network_resources_data_test.cc b/third_party/blink/renderer/core/inspector/network_resources_data_test.cc
new file mode 100644
index 0000000..6c8ce71f
--- /dev/null
+++ b/third_party/blink/renderer/core/inspector/network_resources_data_test.cc
@@ -0,0 +1,111 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/core/inspector/network_resources_data.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/platform/network/encoded_form_data.h"
+#include "third_party/blink/renderer/platform/testing/task_environment.h"
+#include "third_party/blink/renderer/platform/weborigin/kurl.h"
+
+namespace blink {
+
+// Regression test for crbug.com/501878477.
+TEST(NetworkResourcesDataTest, ClearPreservesResourcesAndIds) {
+  test::TaskEnvironment task_environment;
+  // Initialize with small limits to trigger eviction easily.
+  NetworkResourcesData* data =
+      MakeGarbageCollected<NetworkResourcesData>(100, 100);
+
+  String request_id = "request-id";
+  String loader_id = "loader-id";
+  KURL url("http://example.com");
+
+  data->ResourceCreated(request_id, loader_id, url, nullptr);
+  data->SetResourceContent(request_id, "content");
+
+  // Clear with preserved loader ID.
+  data->Clear(loader_id);
+  EXPECT_EQ(data->Resources().size(), 1u);
+
+  // Adding a new large resource should now successfully evict the preserved
+  // resource because its ID was added back to the deque.
+  String new_request_id = "new-request-id";
+  data->ResourceCreated(new_request_id, "new-loader-id", url, nullptr);
+  // Total size = initial_size + 100. Since limit is 100, eviction MUST happen.
+  data->SetResourceContent(
+      new_request_id,
+      String(base::span<const char>(std::vector<char>(100, 'a'))));
+
+  // The first resource should be evicted (content cleared).
+  bool found_evicted = false;
+  for (auto& resource : data->Resources()) {
+    if (resource->RequestId() == request_id) {
+      EXPECT_TRUE(resource->IsContentEvicted());
+      found_evicted = true;
+    }
+  }
+  EXPECT_TRUE(found_evicted);
+}
+
+// Regression test for crbug.com/501878477.
+TEST(NetworkResourcesDataTest, EnsureFreeSpaceSafety) {
+  test::TaskEnvironment task_environment;
+  NetworkResourcesData* data =
+      MakeGarbageCollected<NetworkResourcesData>(100, 100);
+
+  // Manually desynchronize for testing the safety check (if possible, though
+  // our fix for Clear makes it harder to reach this state via public API).
+  // We can just rely on the fact that if we didn't have the Clear fix, this
+  // would crash. With the safety check, even if we were in a bad state, we
+  // won't OOB read.
+
+  // Since we can't easily desynchronize anymore, this test mostly ensures
+  // no regressions in basic eviction.
+  data->ResourceCreated("id1", "loader", KURL("http://a.com"), nullptr);
+  data->SetResourceContent("id1", "content");
+
+  // Trigger immediate eviction if needed
+  data->SetResourcesDataSizeLimits(10, 10);
+
+  // Adding more should work safely.
+  data->ResourceCreated("id2", "loader", KURL("http://b.com"), nullptr);
+  data->SetResourceContent("id2", "small");
+}
+
+// Regression test for crbug.com/501878477.
+TEST(NetworkResourcesDataTest, SetResourcesDataSizeLimitsEvictsCorrectly) {
+  test::TaskEnvironment task_environment;
+  NetworkResourcesData* data =
+      MakeGarbageCollected<NetworkResourcesData>(1000, 1000);
+
+  data->ResourceCreated("id1", "loader", KURL("http://a.com"), nullptr);
+  data->SetResourceContent("id1", "some-data");
+  size_t size1 = data->Resources().front()->ContentSize();
+
+  data->ResourceCreated("id2", "loader", KURL("http://b.com"), nullptr);
+  data->SetResourceContent("id2", "more-data");
+  size_t size2 = 0;
+  for (auto& r : data->Resources()) {
+    if (r->RequestId() == "id2") {
+      size2 = r->ContentSize();
+    }
+  }
+
+  // Reduce limit to be less than size1 + size2.
+  // This should trigger EnsureFreeSpace(0).

diff --git a/third_party/blink/renderer/core/inspector/network_resources_data_test.cc b/third_party/blink/renderer/core/inspector/network_resources_data_test.cc
new file mode 100644
index 0000000..6c8ce71f
--- /dev/null
+++ b/third_party/blink/renderer/core/inspector/network_resources_data_test.cc
@@ -0,0 +1,111 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/core/inspector/network_resources_data.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/platform/network/encoded_form_data.h"
+#include "third_party/blink/renderer/platform/testing/task_environment.h"
+#include "third_party/blink/renderer/platform/weborigin/kurl.h"
+
+namespace blink {
+
+// Regression test for crbug.com/501878477.
+TEST(NetworkResourcesDataTest, ClearPreservesResourcesAndIds) {
+  test::TaskEnvironment task_environment;
+  // Initialize with small limits to trigger eviction easily.
+  NetworkResourcesData* data =
+      MakeGarbageCollected<NetworkResourcesData>(100, 100);
+
+  String request_id = "request-id";
+  String loader_id = "loader-id";
+  KURL url("http://example.com");
+
+  data->ResourceCreated(request_id, loader_id, url, nullptr);
+  data->SetResourceContent(request_id, "content");
+
+  // Clear with preserved loader ID.
+  data->Clear(loader_id);
+  EXPECT_EQ(data->Resources().size(), 1u);
+
+  // Adding a new large resource should now successfully evict the preserved
+  // resource because its ID was added back to the deque.
+  String new_request_id = "new-request-id";
+  data->ResourceCreated(new_request_id, "new-loader-id", url, nullptr);
+  // Total size = initial_size + 100. Since limit is 100, eviction MUST happen.
+  data->SetResourceContent(
+      new_request_id,
+      String(base::span<const char>(std::vector<char>(100, 'a'))));
+
+  // The first resource should be evicted (content cleared).
+  bool found_evicted = false;
+  for (auto& resource : data->Resources()) {
+    if (resource->RequestId() == request_id) {
+      EXPECT_TRUE(resource->IsContentEvicted());
+      found_evicted = true;
+    }
+  }
+  EXPECT_TRUE(found_evicted);
+}
+
+// Regression test for crbug.com/501878477.
+TEST(NetworkResourcesDataTest, EnsureFreeSpaceSafety) {
+  test::TaskEnvironment task_environment;
+  NetworkResourcesData* data =
+      MakeGarbageCollected<NetworkResourcesData>(100, 100);
+
+  // Manually desynchronize for testing the safety check (if possible, though
+  // our fix for Clear makes it harder to reach this state via public API).
+  // We can just rely on the fact that if we didn't have the Clear fix, this
+  // would crash. With the safety check, even if we were in a bad state, we
+  // won't OOB read.
+
+  // Since we can't easily desynchronize anymore, this test mostly ensures
+  // no regressions in basic eviction.
+  data->ResourceCreated("id1", "loader", KURL("http://a.com"), nullptr);
+  data->SetResourceContent("id1", "content");
+
+  // Trigger immediate eviction if needed
+  data->SetResourcesDataSizeLimits(10, 10);
+
+  // Adding more should work safely.
+  data->ResourceCreated("id2", "loader", KURL("http://b.com"), nullptr);
+  data->SetResourceContent("id2", "small");
+}
+
+// Regression test for crbug.com/501878477.
+TEST(NetworkResourcesDataTest, SetResourcesDataSizeLimitsEvictsCorrectly) {
+  test::TaskEnvironment task_environment;
+  NetworkResourcesData* data =
+      MakeGarbageCollected<NetworkResourcesData>(1000, 1000);
+
+  data->ResourceCreated("id1", "loader", KURL("http://a.com"), nullptr);
+  data->SetResourceContent("id1", "some-data");
+  size_t size1 = data->Resources().front()->ContentSize();
+
+  data->ResourceCreated("id2", "loader", KURL("http://b.com"), nullptr);
+  data->SetResourceContent("id2", "more-data");
+  size_t size2 = 0;
+  for (auto& r : data->Resources()) {
+    if (r->RequestId() == "id2") {
+      size2 = r->ContentSize();
+    }
+  }
+
+  // Reduce limit to be less than size1 + size2.
+  // This should trigger EnsureFreeSpace(0).
+  data->SetResourcesDataSizeLimits(size1 + size2 - 1, 1000);
+
+  // At least one resource should be evicted.
+  bool any_evicted = false;
+  for (auto& resource : data->Resources()) {
+    if (resource->IsContentEvicted()) {
+      any_evicted = true;
+      break;
+    }
+  }
+  EXPECT_TRUE(any_evicted);
+}
+
+}  // namespace blink