Critical chrome Input Validation 🔧 Commit mapped

Overview

Critical
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactImproper input validation in FileAPI
DescriptionImproper input validation in FileAPI
ComponentFileAPI
Bug ClassInput Validation
Tracker572193211
Fix commit136b3f96d168 (chromium/src) +334/-188
CISA KEVNot listed
CreditedXinyang Ge (Anthropic), assisted by Claude
Disclosed2026-10-27

Changed Functions

FunctionChangeNotes
if
storage/browser/blob/blob_data_builder.cc
modified
num_building_dependent_blobs
storage/browser/blob/blob_entry.cc
modified
if
storage/browser/blob/blob_entry.cc
modified
for
storage/browser/blob/blob_entry.cc
modified
ShareableBlobDataItem
storage/browser/blob/blob_entry.h
modified
COMPONENT_EXPORT
storage/browser/blob/blob_entry.h
modified
for
storage/browser/blob/blob_storage_context.cc
modified

Files Changed

  • storage/browser/blob/blob_data_handle.cc
  • storage/browser/blob/blob_storage_context.h
  • storage/browser/blob/blob_storage_context.cc
  • storage/browser/blob/blob_storage_context_unittest.cc
  • storage/browser/blob/blob_data_builder.cc
  • storage/browser/blob/view_blob_internals_job.cc
  • storage/browser/blob/blob_entry.cc
  • storage/browser/blob/blob_flattener_unittest.cc
  • storage/browser/blob/blob_storage_registry.h
  • storage/browser/blob/blob_entry.h
  • storage/browser/blob/blob_slice_unittest.cc
  • storage/browser/blob/blob_storage_registry.cc
  • storage/browser/blob/blob_storage_registry_unittest.cc
  • storage/browser/blob/blob_data_builder.h
From 136b3f96d1685548e2fa11fb57a667f6dbdd7087 Mon Sep 17 00:00:00 2001
From: Evan Stade <[email protected]>
Date: Wed, 21 Oct 2026 13:19:50 -0700
Subject: [PATCH] Avoid UAFs in BlobStorageContext

Many different flows could lead to a UAF of a BlobEntry*. The basic
story is:

1. BlobEntry A transitively holds some BlobDataHandle for B

2. Some operation on A, such as FinishBuilding, drops said handle for B

3. B has no more handles and stops using memory, freeing up memory for
some other BlobDataHandle C which had been blocked on quota

4. error occurs while C is acting on this, C self destructs

5. turns out C was the only thing holding A alive; A is destroyed

6. the rest of FinishLoading (or another method) continues, and doesn't
realize `BlobEntry* a` is now deleted.

A small twist on this story: `ClearAndFreeMemory(entry)` was also
re-entrant, because it can trigger a self-deletion similar to the above,
where `ClearAndFreeMemory` is called for the second time in
`DecrementBlobRefCount`. But `ClearAndFreeMemory` is mostly unnecessary
when the BlobEntry is already being deleted, so don't call it explicitly
on `DecrementBlobRefCount`, instead making it effectively part of
`~BlobEntry`.

Since this is a broad class of issues, it's not easy to spot-fix to
produce a small change. Instead of pervasively passing around
`BlobEntry*`, pass around `scoped_refptr<BlobEntry>`, so that it won't
be deleted out from under other operations.

Confusingly, `BlobDataHandle` is already a sort of ref counting, and
`BlobEntry` contains symbol names like `ref_count` and such; those are
renamed to "handle count" instead to avoid confusing with
`base::RefCounted`.

Bug: 572193211
Change-Id: I3d1747482ccfab8ccbff79a49219394c014789c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8620915
Reviewed-by: Fergal Daly <[email protected]>
Commit-Queue: Evan Stade <[email protected]>
Reviewed-by: Eriko Kurimoto <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1724068}
---
diff --git a/storage/browser/blob/blob_data_builder.cc b/storage/browser/blob/blob_data_builder.cc
index c058baa..ab8f165 100644
--- a/storage/browser/blob/blob_data_builder.cc
+++ b/storage/browser/blob/blob_data_builder.cc
@@ -174,7 +174,7 @@
                                  uint64_t length,
                                  const BlobStorageRegistry& blob_registry) {
   DCHECK_GT(length, 0ul);
-  const BlobEntry* ref_entry = blob_registry.GetEntry(uuid);
+  scoped_refptr<const BlobEntry> ref_entry = blob_registry.GetEntry(uuid);
 
   // Self-references or non-existing blob references are invalid.
   if (!ref_entry || uuid == uuid_) {
@@ -242,10 +242,10 @@
     return;
   }
 
-  SliceBlob(ref_entry, offset, length);
+  SliceBlob(std::move(ref_entry), offset, length);
 }
 
-void BlobDataBuilder::SliceBlob(const BlobEntry* source,
+void BlobDataBuilder::SliceBlob(scoped_refptr<const BlobEntry> source,
                                 uint64_t slice_offset,
                                 uint64_t slice_size) {
   const auto& source_items = source->items();
diff --git a/storage/browser/blob/blob_data_builder.h b/storage/browser/blob/blob_data_builder.h
index c0dc7aa2..0a0b0a2 100644
--- a/storage/browser/blob/blob_data_builder.h
+++ b/storage/browser/blob/blob_data_builder.h
@@ -248,7 +248,7 @@
       ::std::ostream* os);
   friend class BlobSliceTest;
 
-  void SliceBlob(const BlobEntry* entry,
+  void SliceBlob(scoped_refptr<const BlobEntry> entry,
                  uint64_t slice_offset,
                  uint64_t slice_size);
 
diff --git a/storage/browser/blob/blob_data_handle.cc b/storage/browser/blob/blob_data_handle.cc
index 777aa9e..1ba116f 100644
--- a/storage/browser/blob/blob_data_handle.cc
+++ b/storage/browser/blob/blob_data_handle.cc
@@ -35,7 +35,7 @@
       content_type_(content_type),
       size_(size),
       context_(context->AsWeakPtr()) {
-  context_->IncrementBlobRefCount(uuid);
+  context_->IncrementBlobHandleCount(uuid);
 }
 
 std::unique_ptr<BlobReader> BlobDataHandle::CreateReader() const {
@@ -44,7 +44,7 @@
 
 BlobDataHandle::BlobDataHandleShared::~BlobDataHandleShared() {
   if (context_.get())
-    context_->DecrementBlobRefCount(uuid_);
+    context_->DecrementBlobHandleCount(uuid_);
 }
 
 BlobDataHandle::BlobDataHandle(const std::string& uuid,
diff --git a/storage/browser/blob/blob_entry.cc b/storage/browser/blob/blob_entry.cc
index 716e9c7..336eab6 100644
--- a/storage/browser/blob/blob_entry.cc
+++ b/storage/browser/blob/blob_entry.cc
@@ -2,13 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "storage/browser/blob/blob_entry.h"
+
 #include <utility>
 
 #include "base/functional/callback.h"
 #include "base/metrics/histogram.h"
+#include "base/task/sequenced_task_runner.h"
 #include "storage/browser/blob/blob_data_handle.h"
 #include "storage/browser/blob/blob_data_item.h"
-#include "storage/browser/blob/blob_entry.h"
 #include "storage/browser/blob/shareable_blob_data_item.h"
 
 namespace storage {
@@ -35,17 +37,19 @@
       num_building_dependent_blobs(num_building_dependent_blobs) {}
 
 BlobEntry::BuildingState::~BuildingState() {
-  DCHECK(!copy_quota_request);
-  DCHECK(!transport_quota_request);
+  CancelRequestsAndAbort();
 }
 
 void BlobEntry::BuildingState::CancelRequestsAndAbort() {
-  if (copy_quota_request)
+  if (copy_quota_request) {
     copy_quota_request->Cancel();
-  if (transport_quota_request)
+  }
+  if (transport_quota_request) {
     transport_quota_request->Cancel();
-  if (build_aborted_callback)
+  }
+  if (build_aborted_callback) {
     std::move(build_aborted_callback).Run();
+  }
 }
 
 BlobEntry::BlobEntry(const std::string& content_type)
@@ -85,12 +89,33 @@
   return items_;
 }
 
-void BlobEntry::ClearItems() {
-  items_.clear();
-}
+void BlobEntry::OnCancelBuilding(BlobStatus reason) {
+  TransportAllowedCallback transport_allowed;
 
-void BlobEntry::ClearOffsets() {
+  if (building_state_) {
+    transport_allowed = std::move(building_state_->transport_allowed_callback);
+
+    if (status_ == BlobStatus::PENDING_CONSTRUCTION) {
+      auto runner = base::SequencedTaskRunner::GetCurrentDefault();
+      for (auto& callback : building_state_->build_started_callbacks) {
+        runner->PostTask(FROM_HERE,
+                         base::BindOnce(std::move(callback), reason));
+      }
+    }
+
+    building_state_->CancelRequestsAndAbort();
+    building_state_->transport_items.clear();
+  }
+
+  items_.clear();
   offsets_.clear();
+  size_ = 0;
+  status_ = reason;
+
+  if (transport_allowed) {
+    std::move(transport_allowed)
+        .Run(reason, std::vector<BlobMemoryController::FileCreationInfo>());
+  }
 }
 
 }  // namespace storage
diff --git a/storage/browser/blob/blob_entry.h b/storage/browser/blob/blob_entry.h
index fb71270..423a7740 100644
--- a/storage/browser/blob/blob_entry.h
+++ b/storage/browser/blob/blob_entry.h
@@ -14,6 +14,7 @@
 #include "base/component_export.h"
 #include "base/functional/callback.h"
 #include "base/memory/raw_ptr.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
 #include "storage/browser/blob/blob_memory_controller.h"
@@ -23,7 +24,8 @@
 class ShareableBlobDataItem;
 
 // Represents a blob in BlobStorageRegistry. Exported only for unit tests.
-class COMPONENT_EXPORT(STORAGE_BROWSER) BlobEntry {
+class COMPONENT_EXPORT(STORAGE_BROWSER) BlobEntry
+    : public base::RefCounted<BlobEntry> {
  public:
   using TransportAllowedCallback = base::OnceCallback<
       void(BlobStatus, std::vector<BlobMemoryController::FileCreationInfo>)>;
@@ -70,11 +72,10 @@
     void CancelRequestsAndAbort();
 
     const bool transport_items_present;
-    // We can have trasnport data that's either populated or unpopulated. If we
+    // We can have transport data that's either populated or unpopulated. If we
     // need population, this is populated.
     TransportAllowedCallback transport_allowed_callback;
-    std::vector<raw_ptr<ShareableBlobDataItem, VectorExperimental>>
-        transport_items;
+    std::vector<raw_ptr<ShareableBlobDataItem>> transport_items;
 
     BuildAbortedCallback build_aborted_callback;
 
@@ -106,8 +107,6 @@
   BlobEntry(const BlobEntry&) = delete;
   BlobEntry& operator=(const BlobEntry&) = delete;
 
-  ~BlobEntry();
-
   // Appends the given shared blob data item to this object.
   void AppendSharedBlobItem(scoped_refptr<ShareableBlobDataItem> item);
   void SetSharedBlobItems(
@@ -123,7 +122,7 @@
 
   BlobStatus status() const { return status_; }
 
-  size_t refcount() const { return refcount_; }
+  size_t handle_count() const { return handle_count_; }
 
   const std::string& content_type() const { return content_type_; }
 
@@ -140,22 +139,25 @@
  protected:
   friend class BlobStorageContext;
 
-  void IncrementRefCount() { ++refcount_; }
-  void DecrementRefCount() { --refcount_; }
+  void IncrementHandleCount() { ++handle_count_; }
+  void DecrementHandleCount() { --handle_count_; }
+
+  void OnCancelBuilding(BlobStatus reason);
 
   void set_status(BlobStatus status) { status_ = status; }
   void set_size(uint64_t size) { size_ = size; }
 
-  void ClearItems();
-  void ClearOffsets();
-
   void set_building_state(std::unique_ptr<BuildingState> building_state) {
     building_state_ = std::move(building_state);
   }
 
  private:
+  friend class base::RefCounted<BlobEntry>;
+
+  ~BlobEntry();
+
   BlobStatus status_ = BlobStatus::PENDING_QUOTA;
-  size_t refcount_ = 0;
+  size_t handle_count_ = 0;
 
   // Metadata.
   std::string content_type_;
diff --git a/storage/browser/blob/blob_storage_context.cc b/storage/browser/blob/blob_storage_context.cc
index a6f7813d..dd51665 100644
--- a/storage/browser/blob/blob_storage_context.cc
+++ b/storage/browser/blob/blob_storage_context.cc
@@ -73,10 +73,10 @@
 
 std::unique_ptr<BlobDataHandle> BlobStorageContext::GetBlobDataFromUUID(
     const std::string& uuid) {
-  BlobEntry* entry = registry_.GetEntry(uuid);
+  scoped_refptr<BlobEntry> entry = registry_.GetEntry(uuid);
   if (!entry)
     return nullptr;
-  return CreateHandle(uuid, entry);
+  return CreateHandle(uuid, std::move(entry));
 }
 
 void BlobStorageContext::GetBlobDataFromBlobRemote(
@@ -112,7 +112,7 @@
     const std::string& content_type,
     std::vector<scoped_refptr<ShareableBlobDataItem>> items) {
   TRACE_EVENT0("Blob", "Context::AddFinishedBlobFromItems");
-  BlobEntry* entry = registry_.CreateEntry(uuid, content_type);
+  scoped_refptr<BlobEntry> entry = registry_.CreateEntry(uuid, content_type);
   for (const auto& item : items) {
     DCHECK_EQ(item->state(), ShareableBlobDataItem::POPULATED_WITH_QUOTA);
     DCHECK_NE(BlobDataItem::Type::kBytesDescription, item->item()->type());
@@ -132,10 +132,10 @@
     BlobStatus reason) {
   DCHECK(!registry_.HasEntry(uuid));
   DCHECK(BlobStatusIsError(reason));
-  BlobEntry* entry = registry_.CreateEntry(uuid, content_type);
+  scoped_refptr<BlobEntry> entry = registry_.CreateEntry(uuid, content_type);
   entry->set_status(reason);
   FinishBuilding(entry);
-  return CreateHandle(uuid, entry);
+  return CreateHandle(uuid, std::move(entry));
 }
 
 std::unique_ptr<BlobDataHandle> BlobStorageContext::AddFutureBlob(
@@ -144,25 +144,24 @@
     BuildAbortedCallback build_aborted_callback) {
   DCHECK(!registry_.HasEntry(uuid));
 
-  BlobEntry* entry = registry_.CreateEntry(uuid, content_type);
+  scoped_refptr<BlobEntry> entry = registry_.CreateEntry(uuid, content_type);
   entry->set_size(blink::BlobUtils::kUnknownSize);
   entry->set_status(BlobStatus::PENDING_CONSTRUCTION);
   entry->set_building_state(std::make_unique<BlobEntry::BuildingState>(
       false, TransportAllowedCallback(), 0));
   entry->building_state_->build_aborted_callback =
       std::move(build_aborted_callback);
-  return CreateHandle(uuid, entry);
+  return CreateHandle(uuid, std::move(entry));
 }
 
 std::unique_ptr<BlobDataHandle> BlobStorageContext::BuildPreregisteredBlob(
     std::unique_ptr<BlobDataBuilder> content,
     TransportAllowedCallback transport_allowed_callback) {
-  BlobEntry* entry = registry_.GetEntry(content->uuid());
-  DCHECK(entry);
+  scoped_refptr<BlobEntry> entry = registry_.GetEntry(content->uuid());
   DCHECK_EQ(BlobStatus::PENDING_CONSTRUCTION, entry->status());
   entry->set_size(0);
 
-  return BuildBlobInternal(entry, std::move(content),
+  return BuildBlobInternal(std::move(entry), std::move(content),
                            std::move(transport_allowed_callback));
 }
 
@@ -171,15 +170,15 @@
     TransportAllowedCallback transport_allowed_callback) {
   DCHECK(!registry_.HasEntry(content->uuid_));
 
-  BlobEntry* entry =
+  scoped_refptr<BlobEntry> entry =
       registry_.CreateEntry(content->uuid(), content->content_type_);
 
-  return BuildBlobInternal(entry, std::move(content),
+  return BuildBlobInternal(std::move(entry), std::move(content),
                            std::move(transport_allowed_callback));
 }
 
 std::unique_ptr<BlobDataHandle> BlobStorageContext::BuildBlobInternal(
-    BlobEntry* entry,
+    scoped_refptr<BlobEntry> entry,
     std::unique_ptr<BlobDataBuilder> content,
     TransportAllowedCallback transport_allowed_callback) {
 #if DCHECK_IS_ON()
@@ -260,8 +259,8 @@
   // data item, and we need to account for that. So we prevent that case by
   // holding onto all blobs.
   for (const std::string& dependent_blob_uuid : content->dependent_blobs()) {
-    BlobEntry* blob_entry = registry_.GetEntry(dependent_blob_uuid);
-    DCHECK(blob_entry);
+    scoped_refptr<BlobEntry> blob_entry =
+        registry_.GetEntry(dependent_blob_uuid);
     if (BlobStatusIsError(blob_entry->status())) {
       entry->set_status(BlobStatus::ERR_REFERENCED_BLOB_BROKEN);
       break;
@@ -275,8 +274,7 @@
     }
   }
 
-  std::vector<raw_ptr<ShareableBlobDataItem, VectorExperimental>>
-      transport_items;
+  std::vector<raw_ptr<ShareableBlobDataItem>> transport_items;
   transport_items.reserve(content->pending_transport_items().size());
   for (const auto& item : content->pending_transport_items())
     transport_items.emplace_back(item.get());
@@ -377,10 +375,10 @@
 }
 
 void BlobStorageContext::NotifyTransportComplete(const std::string& uuid) {
-  BlobEntry* entry = registry_.GetEntry(uuid);
+  scoped_refptr<BlobEntry> entry = registry_.GetEntry(uuid);
   CHECK(entry) << "There is no blob entry with uuid " << uuid;
   DCHECK(BlobStatusIsPending(entry->status()));
-  NotifyTransportCompleteInternal(entry);
+  NotifyTransportCompleteInternal(std::move(entry));
 }
 
... (file diff truncated)
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/storage/browser/blob/blob_flattener_unittest.cc b/storage/browser/blob/blob_flattener_unittest.cc
index 4c2b5ea..8225d5b 100644
--- a/storage/browser/blob/blob_flattener_unittest.cc
+++ b/storage/browser/blob/blob_flattener_unittest.cc
@@ -96,7 +96,7 @@
 
   const ShareableBlobDataItem& GetItemInBlob(const std::string& uuid,
                                              size_t index) {
-    const BlobEntry* entry = registry().GetEntry(uuid);
+    scoped_refptr<const BlobEntry> entry = registry().GetEntry(uuid);
     EXPECT_TRUE(entry);
     return *entry->items()[index];
   }
diff --git a/storage/browser/blob/blob_slice_unittest.cc b/storage/browser/blob/blob_slice_unittest.cc
index a1684a4..563687ec 100644
--- a/storage/browser/blob/blob_slice_unittest.cc
+++ b/storage/browser/blob/blob_slice_unittest.cc
@@ -53,10 +53,10 @@
   }
 
   void Slice(BlobDataBuilder& builder,
-             BlobEntry* source,
+             scoped_refptr<BlobEntry> source,
              uint64_t slice_offset,
              uint64_t slice_size) {
-    builder.SliceBlob(source, slice_offset, slice_size);
+    builder.SliceBlob(std::move(source), slice_offset, slice_size);
   }
 
   void ExpectSlice(scoped_refptr<ShareableBlobDataItem> dest_item,
@@ -76,12 +76,12 @@
 TEST_F(BlobSliceTest, FullItem) {
   const size_t kSize = 5u;
 
-  BlobEntry data(kType);
+  auto data = base::MakeRefCounted<BlobEntry>(kType);
   scoped_refptr<ShareableBlobDataItem> item = CreateDataItem(kSize);
-  data.AppendSharedBlobItem(item);
+  data->AppendSharedBlobItem(item);
 
   BlobDataBuilder builder(kId);
-  Slice(builder, &data, 0, 5);
+  Slice(builder, data, 0, 5);
   EXPECT_TRUE(builder.copies().empty());
   ASSERT_EQ(1u, builder.items().size());
   EXPECT_EQ(item, builder.items()[0]);
@@ -90,12 +90,12 @@
 TEST_F(BlobSliceTest, SliceSingleItem) {
   const size_t kSize = 5u;
 
-  BlobEntry data(kType);
+  auto data = base::MakeRefCounted<BlobEntry>(kType);
   scoped_refptr<ShareableBlobDataItem> item = CreateDataItem(kSize);
-  data.AppendSharedBlobItem(item);
+  data->AppendSharedBlobItem(item);
 
   BlobDataBuilder builder(kId);
-  Slice(builder, &data, 1, 3);
+  Slice(builder, data, 1, 3);
   ASSERT_EQ(1u, builder.copies().size());
   ASSERT_EQ(1u, builder.items().size());
 
@@ -106,14 +106,14 @@
   const size_t kSize1 = 5u;
   const size_t kSize2 = 10u;
 
-  BlobEntry data(kType);
+  auto data = base::MakeRefCounted<BlobEntry>(kType);
   scoped_refptr<ShareableBlobDataItem> item1 = CreateDataItem(kSize1);
   scoped_refptr<ShareableBlobDataItem> item2 = CreateDataItem(kSize2);
-  data.AppendSharedBlobItem(item1);
-  data.AppendSharedBlobItem(item2);
+  data->AppendSharedBlobItem(item1);
+  data->AppendSharedBlobItem(item2);
 
   BlobDataBuilder builder(kId);
-  Slice(builder, &data, 6, 2);
+  Slice(builder, data, 6, 2);
   ASSERT_EQ(1u, builder.copies().size());
   ASSERT_EQ(1u, builder.items().size());
 
@@ -124,14 +124,14 @@
   const size_t kSize1 = 5u;
   const size_t kSize2 = 10u;
 
-  BlobEntry data(kType);
+  auto data = base::MakeRefCounted<BlobEntry>(kType);
   scoped_refptr<ShareableBlobDataItem> item1 = CreateDataItem(kSize1);
   scoped_refptr<ShareableBlobDataItem> item2 = CreateDataItem(kSize2);
-  data.AppendSharedBlobItem(item1);
-  data.AppendSharedBlobItem(item2);
+  data->AppendSharedBlobItem(item1);
+  data->AppendSharedBlobItem(item2);
 
   BlobDataBuilder builder(kId);
-  Slice(builder, &data, 4, 10);
+  Slice(builder, data, 4, 10);
   ASSERT_EQ(2u, builder.copies().size());
   ASSERT_EQ(2u, builder.items().size());
 
@@ -143,14 +143,14 @@
   const size_t kSize1 = 5u;
   const size_t kSize2 = 10u;
 
-  BlobEntry data(kType);
+  auto data = base::MakeRefCounted<BlobEntry>(kType);
   scoped_refptr<ShareableBlobDataItem> item1 = CreateFileItem(0u, kSize1);
   scoped_refptr<ShareableBlobDataItem> item2 = CreateDataItem(kSize2);
-  data.AppendSharedBlobItem(item1);
-  data.AppendSharedBlobItem(item2);
+  data->AppendSharedBlobItem(item1);
+  data->AppendSharedBlobItem(item2);
 
   BlobDataBuilder builder(kId);
-  Slice(builder, &data, 4, 2);
+  Slice(builder, data, 4, 2);
   ASSERT_EQ(1u, builder.copies().size());
   ASSERT_EQ(2u, builder.items().size());
 
@@ -163,16 +163,16 @@
   const size_t kSize2 = 10u;
   const size_t kSize3 = 10u;
 
-  BlobEntry data(kType);
+  auto data = base::MakeRefCounted<BlobEntry>(kType);
   scoped_refptr<ShareableBlobDataItem> item1 = CreateDataItem(kSize1);
   scoped_refptr<ShareableBlobDataItem> item2 = CreateFileItem(0u, kSize2);
   scoped_refptr<ShareableBlobDataItem> item3 = CreateDataItem(kSize3);
-  data.AppendSharedBlobItem(item1);
-  data.AppendSharedBlobItem(item2);
-  data.AppendSharedBlobItem(item3);
+  data->AppendSharedBlobItem(item1);
+  data->AppendSharedBlobItem(item2);
+  data->AppendSharedBlobItem(item3);
 
   BlobDataBuilder builder(kId);
-  Slice(builder, &data, 2, 20);
+  Slice(builder, data, 2, 20);
   ASSERT_EQ(2u, builder.copies().size());
   ASSERT_EQ(3u, builder.items().size());
 
@@ -182,12 +182,12 @@
 }
 
 TEST_F(BlobSliceTest, SliceTempFileItem) {
-  BlobEntry data(kType);
+  auto data = base::MakeRefCounted<BlobEntry>(kType);
   scoped_refptr<ShareableBlobDataItem> item1 = CreateTempFileItem(1u, 10u);
-  data.AppendSharedBlobItem(item1);
+  data->AppendSharedBlobItem(item1);
 
   BlobDataBuilder builder(kId);
-  Slice(builder, &data, 2, 5);
+  Slice(builder, data, 2, 5);
   ASSERT_EQ(1u, builder.copies().size());
   ASSERT_EQ(1u, builder.items().size());
   auto dest_item = builder.items()[0];
diff --git a/storage/browser/blob/blob_storage_context_unittest.cc b/storage/browser/blob/blob_storage_context_unittest.cc
index 7f25ee77..9c49844 100644
--- a/storage/browser/blob/blob_storage_context_unittest.cc
+++ b/storage/browser/blob/blob_storage_context_unittest.cc
@@ -22,6 +22,7 @@
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
+#include "base/test/bind.h"
 #include "base/test/task_environment.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/threading/thread_restrictions.h"
@@ -114,12 +115,12 @@
     context_->mutable_memory_controller()->set_limits_for_testing(limits);
   }
 
-  void IncrementRefCount(const std::string& uuid) {
-    context_->IncrementBlobRefCount(uuid);
+  void IncrementHandleCount(const std::string& uuid) {
+    context_->IncrementBlobHandleCount(uuid);
   }
 
-  void DecrementRefCount(const std::string& uuid) {
-    context_->DecrementBlobRefCount(uuid);
+  void DecrementHandleCount(const std::string& uuid) {
+    context_->DecrementBlobHandleCount(uuid);
   }
 
   std::string UUIDFromBlob(blink::mojom::Blob* blob) {
@@ -301,16 +302,16 @@
   std::unique_ptr<BlobDataHandle> blob_data_handle = SetupBasicBlob(kId);
 
   // Do an extra increment to keep it around after we kill the handle.
-  IncrementRefCount(kId);
-  IncrementRefCount(kId);
-  DecrementRefCount(kId);
+  IncrementHandleCount(kId);
+  IncrementHandleCount(kId);
+  DecrementHandleCount(kId);
   blob_data_handle = context_->GetBlobDataFromUUID(kId);
   EXPECT_TRUE(blob_data_handle);
   blob_data_handle.reset();
   base::RunLoop().RunUntilIdle();
 
   EXPECT_TRUE(context_->registry().HasEntry(kId));
-  DecrementRefCount(kId);
+  DecrementHandleCount(kId);
   EXPECT_FALSE(context_->registry().HasEntry(kId));
 
   // Make sure it goes away in the end.
@@ -972,6 +973,147 @@
   EXPECT_EQ(0u, context_->memory_controller().memory_usage());
 }
 
+// Regression test for crbug.com/572193211
+TEST_F(BlobStorageContextTest, FinishBuildingWontUafEntry) {
+  context_ = std::make_unique<BlobStorageContext>(
+      temp_dir_.GetPath(), temp_dir_.GetPath(), file_runner_);
+  SetTestMemoryLimits();
+
+  const std::string kIdB("id_b");
+  const std::string kIdA("id_a");
+  const std::string kIdC("id_c");
+
+  const size_t a_future_size = kTestBlobStorageMaxBlobMemorySize / 10;
+  const size_t b_future_size = kTestBlobStorageMaxBlobMemorySize * 3 / 4;
+  const size_t c_future_size = kTestBlobStorageMaxBlobMemorySize / 2;
+
+  // 1. Start building Blob B with 300 bytes of future data. Because B is not
+  //    yet populated, its 300 bytes count toward `memory_usage()` (via
+  //    `blob_memory_used_`) without being eligible for eviction to disk
+  //    (`in_flight_memory_used_` stays 0).
+  auto builder_b = std::make_unique<BlobDataBuilder>(kIdB);
+  BlobDataBuilder::FutureData future_data_b =
+      builder_b->AppendFutureData(b_future_size);
+  BlobStatus status_b = BlobStatus::ERR_INVALID_CONSTRUCTION_ARGUMENTS;
+  std::unique_ptr<BlobDataHandle> handle_b = context_->BuildBlob(
+      std::move(builder_b),
+      base::BindOnce(&SaveBlobStatusAndFiles, &status_b, &files_));
+  EXPECT_EQ(BlobStatus::PENDING_TRANSPORT, status_b);
+  EXPECT_EQ(300u, context_->memory_controller().memory_usage());
+
+  // 2. Start building Blob A, which slices from B, so A's
+  // `building_state_->dependent_blobs` holds a handle to B.
+  auto builder_a = std::make_unique<BlobDataBuilder>(kIdA);
+  builder_a->AppendBlob(kIdB, 0, 10, context_->registry());
+  BlobDataBuilder::FutureData future_data_a =
+      builder_a->AppendFutureData(a_future_size);
+  BlobStatus status_a = BlobStatus::ERR_INVALID_CONSTRUCTION_ARGUMENTS;
+  std::unique_ptr<BlobDataHandle> handle_a = context_->BuildBlob(
+      std::move(builder_a),
+      base::BindOnce(&SaveBlobStatusAndFiles, &status_a, &files_));
+  EXPECT_EQ(BlobStatus::PENDING_TRANSPORT, status_a);
+
+  // Drop external handle to B; now ONLY A's `building_state_->dependent_blobs`
+  // keeps B (and its 300-byte MemoryAllocation) alive.
+  handle_b.reset();
+
+  // 3. Start building Blob C, which depends on A and requests 200 bytes of
+  //    transport quota. Because 300 + 20 + 200 > 400, C's quota request is
+  //    queued in `pending_memory_quota_tasks_`.
+  auto builder_c = std::make_unique<BlobDataBuilder>(kIdC);
+  builder_c->AppendBlob(kIdA, 0, 10, context_->registry());
+  builder_c->AppendFutureData(c_future_size);
+  std::unique_ptr<BlobDataHandle> handle_c = context_->BuildBlob(
+      std::move(builder_c),
+      base::BindLambdaForTesting(
+          [&](BlobStatus status, std::vector<FileCreationInfo> /*files*/) {
+            if (status == BlobStatus::PENDING_TRANSPORT) {
+              // Simulate transport setup failure (e.g. data pipe creation OOM).
+              // Canceling C resets C's `building_state_`, dropping the last
+              // handle to A synchronously while A is inside `FinishBuilding`.
+              context_->CancelBuildingBlob(kIdC, BlobStatus::ERR_OUT_OF_MEMORY);
+            }
+          }));
+  EXPECT_EQ(BlobStatus::PENDING_QUOTA, handle_c->GetBlobStatus());
+
+  // Drop external handle to A; now ONLY C's `building_state_->dependent_blobs`
+  // keeps A alive. Keep `handle_c` alive so C stays in the registry while
+  // waiting for quota.
+  handle_a.reset();
+  EXPECT_TRUE(context_->registry().HasEntry(kIdA));
+
+  // 4. Cancel A. Inside `FinishBuilding(A)`:
+  //    - `A->building_state_.reset()` drops A's handle to B.
+  //    - B is deleted, freeing its 300-byte allocation and synchronously
+  //      granting C's 200-byte quota request.
+  //    - C's `transport_allowed_callback` runs and cancels C.
+  //    - C's `building_state_.reset()` drops the last handle to A, deleting A!
+  context_->CancelBuildingBlob(kIdA, BlobStatus::ERR_OUT_OF_MEMORY);
+  EXPECT_FALSE(context_->registry().HasEntry(kIdA));
+}
+
+// Regression test for crbug.com/572193211
+TEST_F(BlobStorageContextTest, CancelBuildingBlobWontUafEntry) {
+  context_ = std::make_unique<BlobStorageContext>(
+      temp_dir_.GetPath(), temp_dir_.GetPath(), file_runner_);
+  SetTestMemoryLimits();
+
+  const std::string kIdA("id_a");
+  const std::string kIdC("id_c");
+
+  const size_t a_future_size = kTestBlobStorageMaxBlobMemorySize * 3 / 4;
... (truncated)
Loading diff…

Original Bug Report

The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.