chore: cherry-pick 2b978fb482 from chromium (#32234)

2026-04-10 03:01:51 -04:00 · 2022-01-04 02:10:26 +01:00
parent 9b94d70194
commit 316f0bc845
2 changed files with 421 additions and 0 deletions
--- a/patches/chromium/.patches
+++ b/patches/chromium/.patches
@@ -169,3 +169,4 @@ allow_null_skbitmap_to_be_transferred_across_threads.patch
 use_weakptrs_for_the_threadediconloader_s_background_tasks.patch
 cachestorage_store_partial_opaque_responses.patch
 cherry-pick-a5f54612590d.patch
+m96_fileapi_move_origin_checks_in_bloburlstore_sooner.patch
--- a/patches/chromium/m96_fileapi_move_origin_checks_in_bloburlstore_sooner.patch
+++ b/patches/chromium/m96_fileapi_move_origin_checks_in_bloburlstore_sooner.patch
@@ -0,0 +1,420 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Marijn Kruisselbrink <mek@chromium.org>
+Date: Tue, 16 Nov 2021 22:16:15 +0000
+Subject: M96: [FileAPI] Move origin checks in BlobURLStore sooner.
+
+Rather than waiting to verify if a valid origin was passed in until
+register/revoke time, check if the origin is valid at the time the
+mojo interface is requested. This avoids the need to pass the
+delegate on to BlobURLStoreImpl, further decoupling this from
+BlobRegistryImpl.
+
+(cherry picked from commit 15cfa2bed3ce9dcdd0a06d3cd3b8330e3591acdc)
+
+Bug: 1262183
+Change-Id: I4889685d03501158abfe4f87c647dc468d005f78
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3264353
+Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
+Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
+Cr-Original-Commit-Position: refs/heads/main@{#940015}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3285385
+Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
+Cr-Commit-Position: refs/branch-heads/4664@{#1078}
+Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512}
+
+diff --git a/chrome/browser/chrome_security_exploit_browsertest.cc b/chrome/browser/chrome_security_exploit_browsertest.cc
+index fd8ec071c4aa52ed59e9c9dbad5b813840ee2510..2ffc8f4b29e2573ed862468ff34b26190dac1372 100644
+--- a/chrome/browser/chrome_security_exploit_browsertest.cc
+++ b/chrome/browser/chrome_security_exploit_browsertest.cc
+@@ -482,8 +482,8 @@ IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTestMojoBlobURLs,
+ 
+   // If the process is killed, this test passes.
+   EXPECT_EQ(
+-      "Received bad user message: Non committable URL passed to "
+-      "BlobURLStore::Register",
+      "Received bad user message: "
+      "URL with invalid origin passed to BlobURLStore::Register",
+       crash_observer.Wait());
+ }
+ 
+@@ -518,7 +518,7 @@ IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTestMojoBlobURLs,
+ 
+   // If the process is killed, this test passes.
+   EXPECT_EQ(
+-      "Received bad user message: Non committable URL passed to "
+-      "BlobURLStore::Register",
+      "Received bad user message: "
+      "URL with invalid origin passed to BlobURLStore::Register",
+       crash_observer.Wait());
+ }
+diff --git a/content/browser/blob_storage/blob_url_unittest.cc b/content/browser/blob_storage/blob_url_unittest.cc
+index 0a7e8fb9285f0efcfdedfab1ecb70eb7fa61cfdf..9c2bac10bbb76c44adfa52d27d00151538ae336a 100644
+--- a/content/browser/blob_storage/blob_url_unittest.cc
+++ b/content/browser/blob_storage/blob_url_unittest.cc
+@@ -182,15 +182,14 @@ class BlobURLTest : public testing::Test {
+ 
+   void TestRequest(const std::string& method,
+                    const net::HttpRequestHeaders& extra_headers) {
+-    GURL url("blob:blah");
+    auto origin = url::Origin::Create(GURL("https://example.com"));
+    auto url = GURL("blob:" + origin.Serialize() + "/id1");
+     network::ResourceRequest request;
+     request.url = url;
+     request.method = method;
+     request.headers = extra_headers;
+ 
+-    storage::MockBlobRegistryDelegate delegate;
+-    storage::BlobURLStoreImpl url_store(blob_url_registry_.AsWeakPtr(),
+-                                        &delegate);
+    storage::BlobURLStoreImpl url_store(origin, blob_url_registry_.AsWeakPtr());
+ 
+     mojo::PendingRemote<blink::mojom::Blob> blob_remote;
+     storage::BlobImpl::Create(
+diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
+index 1fd9f1ba1b07e4c890146f5b29bdff584e0fc95b..a436c9d5533800adf0551e97ba874cbb4498209c 100644
+--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
+@@ -679,7 +679,7 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTestMojoBlobURLs,
+   // If the process is killed, this test passes.
+   EXPECT_EQ(
+       "Received bad user message: "
+-      "Non committable URL passed to BlobURLStore::Register",
+      "URL with invalid origin passed to BlobURLStore::Register",
+       crash_observer.Wait());
+ }
+ 
+diff --git a/storage/browser/blob/blob_registry_impl.cc b/storage/browser/blob/blob_registry_impl.cc
+index 34e736e80de6093dd400e4dae21d8ff4ff3180d6..2479803044e63b20964977eda75f89da31fb9529 100644
+--- a/storage/browser/blob/blob_registry_impl.cc
+++ b/storage/browser/blob/blob_registry_impl.cc
+@@ -629,13 +629,14 @@ void BlobRegistryImpl::GetBlobFromUUID(
+ void BlobRegistryImpl::URLStoreForOrigin(
+     const url::Origin& origin,
+     mojo::PendingAssociatedReceiver<blink::mojom::BlobURLStore> receiver) {
+-  // TODO(mek): Pass origin on to BlobURLStoreImpl so it can use it to generate
+-  // Blob URLs, and verify at this point that the renderer can create URLs for
+-  // that origin.
+   Delegate* delegate = receivers_.current_context().get();
+   DCHECK(delegate);
+  if (!origin.opaque() && !delegate->CanCommitURL(origin.GetURL())) {
+    mojo::ReportBadMessage(
+        "Non committable origin passed to BlobRegistryImpl::URLStoreForOrigin");
+  }
+   auto self_owned_associated_receiver = mojo::MakeSelfOwnedAssociatedReceiver(
+-      std::make_unique<BlobURLStoreImpl>(url_registry_, delegate),
+      std::make_unique<BlobURLStoreImpl>(origin, url_registry_),
+       std::move(receiver));
+   if (g_url_store_creation_hook)
+     g_url_store_creation_hook->Run(self_owned_associated_receiver);
+diff --git a/storage/browser/blob/blob_url_store_impl.cc b/storage/browser/blob/blob_url_store_impl.cc
+index b46ee60849c9b758e022d7e576de61b080fd59f5..d4edd988f5982788f5157d27fbfa31abcb1eede7 100644
+--- a/storage/browser/blob/blob_url_store_impl.cc
+++ b/storage/browser/blob/blob_url_store_impl.cc
+@@ -5,6 +5,7 @@
+ #include "storage/browser/blob/blob_url_store_impl.h"
+ 
+ #include "base/bind.h"
+#include "base/strings/strcat.h"
+ #include "mojo/public/cpp/bindings/receiver_set.h"
+ #include "storage/browser/blob/blob_impl.h"
+ #include "storage/browser/blob/blob_url_loader_factory.h"
+@@ -58,9 +59,9 @@ class BlobURLTokenImpl : public blink::mojom::BlobURLToken {
+   const base::UnguessableToken token_;
+ };
+ 
+-BlobURLStoreImpl::BlobURLStoreImpl(base::WeakPtr<BlobUrlRegistry> registry,
+-                                   BlobRegistryImpl::Delegate* delegate)
+-    : registry_(std::move(registry)), delegate_(delegate) {}
+BlobURLStoreImpl::BlobURLStoreImpl(const url::Origin& origin,
+                                   base::WeakPtr<BlobUrlRegistry> registry)
+    : origin_(origin), registry_(std::move(registry)) {}
+ 
+ BlobURLStoreImpl::~BlobURLStoreImpl() {
+   if (registry_) {
+@@ -72,20 +73,9 @@ BlobURLStoreImpl::~BlobURLStoreImpl() {
+ void BlobURLStoreImpl::Register(mojo::PendingRemote<blink::mojom::Blob> blob,
+                                 const GURL& url,
+                                 RegisterCallback callback) {
+-  if (!url.SchemeIsBlob()) {
+-    mojo::ReportBadMessage("Invalid scheme passed to BlobURLStore::Register");
+-    std::move(callback).Run();
+-    return;
+-  }
+-  if (!delegate_->CanCommitURL(url)) {
+-    mojo::ReportBadMessage(
+-        "Non committable URL passed to BlobURLStore::Register");
+-    std::move(callback).Run();
+-    return;
+-  }
+-  if (BlobUrlUtils::UrlHasFragment(url)) {
+-    mojo::ReportBadMessage(
+-        "URL with fragment passed to BlobURLStore::Register");
+  // TODO(mek): Generate blob URLs here, rather than validating the URLs the
+  // renderer process generated.
+  if (!BlobUrlIsValid(url, "Register")) {
+     std::move(callback).Run();
+     return;
+   }
+@@ -97,19 +87,8 @@ void BlobURLStoreImpl::Register(mojo::PendingRemote<blink::mojom::Blob> blob,
+ }
+ 
+ void BlobURLStoreImpl::Revoke(const GURL& url) {
+-  if (!url.SchemeIsBlob()) {
+-    mojo::ReportBadMessage("Invalid scheme passed to BlobURLStore::Revoke");
+-    return;
+-  }
+-  if (!delegate_->CanCommitURL(url)) {
+-    mojo::ReportBadMessage(
+-        "Non committable URL passed to BlobURLStore::Revoke");
+  if (!BlobUrlIsValid(url, "Revoke"))
+     return;
+-  }
+-  if (BlobUrlUtils::UrlHasFragment(url)) {
+-    mojo::ReportBadMessage("URL with fragment passed to BlobURLStore::Revoke");
+-    return;
+-  }
+ 
+   if (registry_)
+     registry_->RemoveUrlMapping(url);
+@@ -144,4 +123,38 @@ void BlobURLStoreImpl::ResolveForNavigation(
+   new BlobURLTokenImpl(registry_, url, std::move(blob), std::move(token));
+ }
+ 
+bool BlobURLStoreImpl::BlobUrlIsValid(const GURL& url,
+                                      const char* method) const {
+  if (!url.SchemeIsBlob()) {
+    mojo::ReportBadMessage(
+        base::StrCat({"Invalid scheme passed to BlobURLStore::", method}));
+    return false;
+  }
+  url::Origin url_origin = url::Origin::Create(url);
+  // For file:// origins blink sometimes creates blob URLs with "null" as origin
+  // and other times "file://" (based on a runtime setting). On the other hand,
+  // `origin_` will always be a non-opaque file: origin for pages loaded from
+  // file:// URLs. To deal with this, we treat file:// origins and
+  // opaque origins separately from non-opaque origins.
+  bool valid_origin = true;
+  if (url_origin.scheme() == url::kFileScheme) {
+    valid_origin = origin_.scheme() == url::kFileScheme;
+  } else if (url_origin.opaque()) {
+    valid_origin = origin_.opaque() || origin_.scheme() == url::kFileScheme;
+  } else {
+    valid_origin = origin_ == url_origin;
+  }
+  if (!valid_origin) {
+    mojo::ReportBadMessage(base::StrCat(
+        {"URL with invalid origin passed to BlobURLStore::", method}));
+    return false;
+  }
+  if (BlobUrlUtils::UrlHasFragment(url)) {
+    mojo::ReportBadMessage(
+        base::StrCat({"URL with fragment passed to BlobURLStore::", method}));
+    return false;
+  }
+  return true;
+}
+
+ }  // namespace storage
+diff --git a/storage/browser/blob/blob_url_store_impl.h b/storage/browser/blob/blob_url_store_impl.h
+index 6b4a738a4646f8d8e35e8bb1be5aba391e612917..5db371d7c6d50520d8b40a3b4cb6d6bbf920b570 100644
+--- a/storage/browser/blob/blob_url_store_impl.h
+++ b/storage/browser/blob/blob_url_store_impl.h
+@@ -11,8 +11,9 @@
+ #include "mojo/public/cpp/bindings/pending_receiver.h"
+ #include "mojo/public/cpp/bindings/pending_remote.h"
+ #include "mojo/public/cpp/bindings/remote.h"
+-#include "storage/browser/blob/blob_registry_impl.h"
+#include "storage/browser/blob/blob_url_registry.h"
+ #include "third_party/blink/public/mojom/blob/blob_url_store.mojom.h"
+#include "url/origin.h"
+ 
+ namespace storage {
+ 
+@@ -21,8 +22,9 @@ class BlobUrlRegistry;
+ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLStoreImpl
+     : public blink::mojom::BlobURLStore {
+  public:
+-  BlobURLStoreImpl(base::WeakPtr<BlobUrlRegistry> registry,
+-                   BlobRegistryImpl::Delegate* delegate);
+  BlobURLStoreImpl(const url::Origin& origin,
+                   base::WeakPtr<BlobUrlRegistry> registry);
+
+   ~BlobURLStoreImpl() override;
+ 
+   void Register(mojo::PendingRemote<blink::mojom::Blob> blob,
+@@ -39,8 +41,12 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLStoreImpl
+       mojo::PendingReceiver<blink::mojom::BlobURLToken> token) override;
+ 
+  private:
+  // Checks if the passed in url is a valid blob url for this blob url store.
+  // Returns false and reports a bad mojo message if not.
+  bool BlobUrlIsValid(const GURL& url, const char* method) const;
+
+  const url::Origin origin_;
+   base::WeakPtr<BlobUrlRegistry> registry_;
+-  BlobRegistryImpl::Delegate* delegate_;
+ 
+   std::set<GURL> urls_;
+ 
+diff --git a/storage/browser/blob/blob_url_store_impl_unittest.cc b/storage/browser/blob/blob_url_store_impl_unittest.cc
+index 69428d0adebbc615fbeedb6b097d43ec6d513e2c..52d17152b3841ea655efd22d3c0a277762994d03 100644
+--- a/storage/browser/blob/blob_url_store_impl_unittest.cc
+++ b/storage/browser/blob/blob_url_store_impl_unittest.cc
+@@ -17,7 +17,6 @@
+ #include "storage/browser/blob/blob_impl.h"
+ #include "storage/browser/blob/blob_storage_context.h"
+ #include "storage/browser/blob/blob_url_registry.h"
+-#include "storage/browser/test/mock_blob_registry_delegate.h"
+ #include "testing/gtest/include/gtest/gtest.h"
+ 
+ using blink::mojom::BlobURLStore;
+@@ -69,9 +68,9 @@ class BlobURLStoreImplTest : public testing::Test {
+ 
+   mojo::PendingRemote<BlobURLStore> CreateURLStore() {
+     mojo::PendingRemote<BlobURLStore> result;
+-    mojo::MakeSelfOwnedReceiver(std::make_unique<BlobURLStoreImpl>(
+-                                    url_registry_.AsWeakPtr(), &delegate_),
+-                                result.InitWithNewPipeAndPassReceiver());
+    mojo::MakeSelfOwnedReceiver(
+        std::make_unique<BlobURLStoreImpl>(kOrigin, url_registry_.AsWeakPtr()),
+        result.InitWithNewPipeAndPassReceiver());
+     return result;
+   }
+ 
+@@ -101,15 +100,19 @@ class BlobURLStoreImplTest : public testing::Test {
+   }
+ 
+   const std::string kId = "id";
+-  const GURL kValidUrl = GURL("blob:id");
+  const url::Origin kOrigin = url::Origin::Create(GURL("https://example.com"));
+  const GURL kValidUrl = GURL("blob:" + kOrigin.Serialize() + "/id1");
+  const GURL kValidUrl2 = GURL("blob:" + kOrigin.Serialize() + "/id2");
+   const GURL kInvalidUrl = GURL("bolb:id");
+-  const GURL kFragmentUrl = GURL("blob:id#fragment");
+  const GURL kFragmentUrl = GURL(kValidUrl.spec() + "#fragment");
+  const url::Origin kWrongOrigin =
+      url::Origin::Create(GURL("https://test.com"));
+  const GURL kWrongOriginUrl = GURL("blob:" + kWrongOrigin.Serialize() + "/id");
+ 
+  protected:
+   base::test::TaskEnvironment task_environment_;
+   std::unique_ptr<BlobStorageContext> context_;
+   BlobUrlRegistry url_registry_;
+-  MockBlobRegistryDelegate delegate_;
+   std::vector<std::string> bad_messages_;
+ };
+ 
+@@ -118,7 +121,7 @@ TEST_F(BlobURLStoreImplTest, BasicRegisterRevoke) {
+       CreateBlobFromString(kId, "hello world");
+ 
+   // Register a URL and make sure the URL keeps the blob alive.
+-  BlobURLStoreImpl url_store(url_registry_.AsWeakPtr(), &delegate_);
+  BlobURLStoreImpl url_store(kOrigin, url_registry_.AsWeakPtr());
+   RegisterURL(&url_store, std::move(blob), kValidUrl);
+ 
+   blob = url_registry_.GetBlobFromUrl(kValidUrl);
+@@ -146,15 +149,13 @@ TEST_F(BlobURLStoreImplTest, RegisterInvalidScheme) {
+   EXPECT_EQ(1u, bad_messages_.size());
+ }
+ 
+-TEST_F(BlobURLStoreImplTest, RegisterCantCommit) {
+TEST_F(BlobURLStoreImplTest, RegisterWrongOrigin) {
+   mojo::PendingRemote<blink::mojom::Blob> blob =
+       CreateBlobFromString(kId, "hello world");
+ 
+-  delegate_.can_commit_url_result = false;
+-
+   mojo::Remote<BlobURLStore> url_store(CreateURLStore());
+-  RegisterURL(url_store.get(), std::move(blob), kValidUrl);
+-  EXPECT_FALSE(url_registry_.GetBlobFromUrl(kValidUrl));
+  RegisterURL(url_store.get(), std::move(blob), kWrongOriginUrl);
+  EXPECT_FALSE(url_registry_.GetBlobFromUrl(kWrongOriginUrl));
+   EXPECT_EQ(1u, bad_messages_.size());
+ }
+ 
+@@ -169,14 +170,13 @@ TEST_F(BlobURLStoreImplTest, RegisterUrlFragment) {
+ }
+ 
+ TEST_F(BlobURLStoreImplTest, ImplicitRevoke) {
+-  const GURL kValidUrl2("blob:id2");
+   mojo::Remote<blink::mojom::Blob> blob(
+       CreateBlobFromString(kId, "hello world"));
+   mojo::PendingRemote<blink::mojom::Blob> blob2;
+   blob->Clone(blob2.InitWithNewPipeAndPassReceiver());
+ 
+   auto url_store =
+-      std::make_unique<BlobURLStoreImpl>(url_registry_.AsWeakPtr(), &delegate_);
+      std::make_unique<BlobURLStoreImpl>(kOrigin, url_registry_.AsWeakPtr());
+   RegisterURL(url_store.get(), blob.Unbind(), kValidUrl);
+   EXPECT_TRUE(url_registry_.GetBlobFromUrl(kValidUrl));
+   RegisterURL(url_store.get(), std::move(blob2), kValidUrl2);
+@@ -192,8 +192,8 @@ TEST_F(BlobURLStoreImplTest, RevokeThroughDifferentURLStore) {
+   mojo::PendingRemote<blink::mojom::Blob> blob =
+       CreateBlobFromString(kId, "hello world");
+ 
+-  BlobURLStoreImpl url_store1(url_registry_.AsWeakPtr(), &delegate_);
+-  BlobURLStoreImpl url_store2(url_registry_.AsWeakPtr(), &delegate_);
+  BlobURLStoreImpl url_store1(kOrigin, url_registry_.AsWeakPtr());
+  BlobURLStoreImpl url_store2(kOrigin, url_registry_.AsWeakPtr());
+ 
+   RegisterURL(&url_store1, std::move(blob), kValidUrl);
+   EXPECT_TRUE(url_registry_.GetBlobFromUrl(kValidUrl));
+@@ -209,11 +209,9 @@ TEST_F(BlobURLStoreImplTest, RevokeInvalidScheme) {
+   EXPECT_EQ(1u, bad_messages_.size());
+ }
+ 
+-TEST_F(BlobURLStoreImplTest, RevokeCantCommit) {
+-  delegate_.can_commit_url_result = false;
+-
+TEST_F(BlobURLStoreImplTest, RevokeWrongOrigin) {
+   mojo::Remote<BlobURLStore> url_store(CreateURLStore());
+-  url_store->Revoke(kValidUrl);
+  url_store->Revoke(kWrongOriginUrl);
+   url_store.FlushForTesting();
+   EXPECT_EQ(1u, bad_messages_.size());
+ }
+@@ -229,7 +227,7 @@ TEST_F(BlobURLStoreImplTest, Resolve) {
+   mojo::PendingRemote<blink::mojom::Blob> blob =
+       CreateBlobFromString(kId, "hello world");
+ 
+-  BlobURLStoreImpl url_store(url_registry_.AsWeakPtr(), &delegate_);
+  BlobURLStoreImpl url_store(kOrigin, url_registry_.AsWeakPtr());
+   RegisterURL(&url_store, std::move(blob), kValidUrl);
+ 
+   blob = ResolveURL(&url_store, kValidUrl);
+@@ -244,7 +242,7 @@ TEST_F(BlobURLStoreImplTest, Resolve) {
+ }
+ 
+ TEST_F(BlobURLStoreImplTest, ResolveNonExistentURL) {
+-  BlobURLStoreImpl url_store(url_registry_.AsWeakPtr(), &delegate_);
+  BlobURLStoreImpl url_store(kOrigin, url_registry_.AsWeakPtr());
+ 
+   mojo::PendingRemote<blink::mojom::Blob> blob =
+       ResolveURL(&url_store, kValidUrl);
+@@ -254,7 +252,7 @@ TEST_F(BlobURLStoreImplTest, ResolveNonExistentURL) {
+ }
+ 
+ TEST_F(BlobURLStoreImplTest, ResolveInvalidURL) {
+-  BlobURLStoreImpl url_store(url_registry_.AsWeakPtr(), &delegate_);
+  BlobURLStoreImpl url_store(kOrigin, url_registry_.AsWeakPtr());
+ 
+   mojo::PendingRemote<blink::mojom::Blob> blob =
+       ResolveURL(&url_store, kInvalidUrl);
+@@ -265,7 +263,7 @@ TEST_F(BlobURLStoreImplTest, ResolveAsURLLoaderFactory) {
+   mojo::PendingRemote<blink::mojom::Blob> blob =
+       CreateBlobFromString(kId, "hello world");
+ 
+-  BlobURLStoreImpl url_store(url_registry_.AsWeakPtr(), &delegate_);
+  BlobURLStoreImpl url_store(kOrigin, url_registry_.AsWeakPtr());
+   RegisterURL(&url_store, std::move(blob), kValidUrl);
+ 
+   mojo::Remote<network::mojom::URLLoaderFactory> factory;
+@@ -291,7 +289,7 @@ TEST_F(BlobURLStoreImplTest, ResolveForNavigation) {
+   mojo::PendingRemote<blink::mojom::Blob> blob =
+       CreateBlobFromString(kId, "hello world");
+ 
+-  BlobURLStoreImpl url_store(url_registry_.AsWeakPtr(), &delegate_);
+  BlobURLStoreImpl url_store(kOrigin, url_registry_.AsWeakPtr());
+   RegisterURL(&url_store, std::move(blob), kValidUrl);
+ 
+   mojo::Remote<blink::mojom::BlobURLToken> token_remote;