diff --git a/build/bromite_patches_list.txt b/build/bromite_patches_list.txt index 177acc6a..2f8bb426 100644 --- a/build/bromite_patches_list.txt +++ b/build/bromite_patches_list.txt @@ -197,6 +197,4 @@ Disable-PrivacyGuide.patch Re-introduce-modal-dialog-flag-to-close-all-tabs.patch sharing-hub-always-use-visible-URL.patch Re-introduce-kWebAuthCable.patch -Add-new-cache-check-function.patch -Stop-cross-origin-cache-hits.patch Revert-clipboard-user-gesture-requirement-removal.patch diff --git a/build/patches/Add-new-cache-check-function.patch b/build/patches/Add-new-cache-check-function.patch deleted file mode 100644 index 20ee6c65..00000000 --- a/build/patches/Add-new-cache-check-function.patch +++ /dev/null @@ -1,157 +0,0 @@ -From: Ari Chivukula -Date: Wed, 10 Aug 2022 23:41:51 +0000 -Subject: Add new cache check function - -Currently, if a.com is loaded and has a favicon at a.com/icon.png and -then b.com is loaded and has the exact same favicon, the cache entry is -shared which permits b.com to notice that a.com was visited. The end -goal of this task is to prevent cross-origin cache leaks. - -This CL adds a new variant of GetFaviconIDForFaviconURL that filters -results by page origin. This will be used in UpdateFaviconMappingsAndFetch -in the next CL, but is just tested here. - -This CL is part of a series: -(1) Cache browser test -(2) Add new cache check function -(3) Stop cross-origin cache hits - -Bug: 1300214 -Change-Id: Ic1513c63f0a09a32e3316d3569f0719990be833b -Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3822854 -Reviewed-by: Scott Violet -Auto-Submit: Ari Chivukula -Commit-Queue: Ari Chivukula -Cr-Commit-Position: refs/heads/main@{#1033779} - -License: GPL-3.0-only - https://spdx.org/licenses/GPL-3.0-only.html ---- - components/favicon/core/favicon_database.cc | 26 ++++++++ - components/favicon/core/favicon_database.h | 10 ++++ - .../favicon/core/favicon_database_unittest.cc | 59 +++++++++++++++++++ - 3 files changed, 95 insertions(+) - -diff --git a/components/favicon/core/favicon_database.cc b/components/favicon/core/favicon_database.cc ---- a/components/favicon/core/favicon_database.cc -+++ b/components/favicon/core/favicon_database.cc -@@ -693,6 +693,32 @@ favicon_base::FaviconID FaviconDatabase::GetFaviconIDForFaviconURL( - return 0; - } - -+favicon_base::FaviconID FaviconDatabase::GetFaviconIDForFaviconURL( -+ const GURL& icon_url, -+ favicon_base::IconType icon_type) { -+ const url::Origin& page_origin) { -+ // Look to see if there even is any relevant cached entry. -+ auto const icon_id = GetFaviconIDForFaviconURL(icon_url, icon_type); -+ if (!icon_id) { -+ return icon_id; -+ } -+ -+ // Check existing mappings to see if any are for the same origin. -+ sql::Statement statement(db_.GetCachedStatement( -+ SQL_FROM_HERE, "SELECT page_url FROM icon_mapping WHERE icon_id=?")); -+ statement.BindInt64(0, icon_id); -+ while (statement.Step()) { -+ const auto candidate_origin = -+ url::Origin::Create(GURL(statement.ColumnString(0))); -+ if (candidate_origin == page_origin) { -+ return icon_id; -+ } -+ } -+ -+ // Act as if there is no entry in the cache if no mapping exists. -+ return 0; -+} -+ - favicon_base::FaviconID FaviconDatabase::GetFaviconIDForFaviconURL( - const GURL& icon_url, - favicon_base::IconType icon_type) { -diff --git a/components/favicon/core/favicon_database.h b/components/favicon/core/favicon_database.h ---- a/components/favicon/core/favicon_database.h -+++ b/components/favicon/core/favicon_database.h -@@ -158,6 +158,16 @@ class FaviconDatabase { - favicon_base::IconType icon_type, - const url::Origin& page_origin); - -+ // Returns the id of the entry in the favicon database with the specified -+ // `icon_url` and `icon_type` (and 0 if no entry exists). This function does -+ // not respect cross-origin partitioning and returns an entry from the cache -+ // without verifying it was stored for the origin requesting it. This can leak -+ // navigation history, see crbug.com/1300214 for more context. -+ favicon_base::FaviconID GetFaviconIDForFaviconURL( -+ const GURL& icon_url, -+ favicon_base::IconType icon_type, -+ const url::Origin& page_origin); -+ - // Returns the id of the entry in the favicon database with the specified - // `icon_url` and `icon_type` (and 0 if no entry exists). This function does - // not respect cross-origin partitioning and returns an entry from the cache -diff --git a/components/favicon/core/favicon_database_unittest.cc b/components/favicon/core/favicon_database_unittest.cc ---- a/components/favicon/core/favicon_database_unittest.cc -+++ b/components/favicon/core/favicon_database_unittest.cc -@@ -1508,3 +1508,62 @@ TEST_F(FaviconDatabaseTest, GetFaviconIDForFaviconURLOriginFilter) { - } - - } // namespace favicon -+TEST_F(FaviconDatabaseTest, GetFaviconIDForFaviconURLOriginFilter) { -+ // Setup DB with `kPageUrl1` mapped to `kIconUrl1`. -+ FaviconDatabase db; -+ ASSERT_EQ(sql::INIT_OK, db.Init(file_name_)); -+ db.BeginTransaction(); -+ scoped_refptr favicon1( -+ new base::RefCountedStaticMemory(kBlob1, sizeof(kBlob1))); -+ const auto icon_id = db.AddFavicon( -+ kIconUrl1, favicon_base::IconType::kFavicon, favicon1, -+ FaviconBitmapType::ON_VISIT, base::Time::Now(), gfx::Size()); -+ db.AddIconMapping(kPageUrl1, icon_id); -+ ASSERT_NE(0, icon_id); -+ -+ // We should be able to find the `icon_id` via the non-filtered function. -+ auto icon_id_found = -+ db.GetFaviconIDForFaviconURL(kIconUrl1, favicon_base::IconType::kFavicon); -+ ASSERT_EQ(icon_id, icon_id_found); -+ -+ // We should be able to find the `icon_id` via a the origin of `kPageUrl1`. -+ icon_id_found = -+ db.GetFaviconIDForFaviconURL(kIconUrl1, favicon_base::IconType::kFavicon, -+ url::Origin::Create(kPageUrl1)); -+ ASSERT_EQ(icon_id, icon_id_found); -+ -+ // We shouldn't be able to find the `icon_id` via a the origin of `kPageUrl2`. -+ icon_id_found = -+ db.GetFaviconIDForFaviconURL(kIconUrl1, favicon_base::IconType::kFavicon, -+ url::Origin::Create(kPageUrl2)); -+ ASSERT_EQ(0, icon_id_found); -+ -+ // We shouldn't be able to find the `icon_id` via a the origin of `kPageUrl3`. -+ icon_id_found = -+ db.GetFaviconIDForFaviconURL(kIconUrl1, favicon_base::IconType::kFavicon, -+ url::Origin::Create(kPageUrl3)); -+ ASSERT_EQ(0, icon_id_found); -+ -+ // If we map `kPageUrl2` then the situation changes. -+ db.AddIconMapping(kPageUrl2, icon_id); -+ -+ // We should be able to find the `icon_id` via a the origin of `kPageUrl1`. -+ icon_id_found = -+ db.GetFaviconIDForFaviconURL(kIconUrl1, favicon_base::IconType::kFavicon, -+ url::Origin::Create(kPageUrl1)); -+ ASSERT_EQ(icon_id, icon_id_found); -+ -+ // We should be able to find the `icon_id` via a the origin of `kPageUrl2`. -+ icon_id_found = -+ db.GetFaviconIDForFaviconURL(kIconUrl1, favicon_base::IconType::kFavicon, -+ url::Origin::Create(kPageUrl2)); -+ ASSERT_EQ(icon_id, icon_id_found); -+ -+ // We shouldn't be able to find the `icon_id` via a the origin of `kPageUrl3`. -+ icon_id_found = -+ db.GetFaviconIDForFaviconURL(kIconUrl1, favicon_base::IconType::kFavicon, -+ url::Origin::Create(kPageUrl3)); -+ ASSERT_EQ(0, icon_id_found); -+} -+ -+} // namespace favicon --- -2.25.1 diff --git a/build/patches/Stop-cross-origin-cache-hits.patch b/build/patches/Stop-cross-origin-cache-hits.patch deleted file mode 100644 index 8cf58eca..00000000 --- a/build/patches/Stop-cross-origin-cache-hits.patch +++ /dev/null @@ -1,43 +0,0 @@ -From: Ari Chivukula -Date: Thu, 11 Aug 2022 00:39:04 +0000 -Subject: Stop cross-origin cache hits - -Currently, if a.com is loaded and has a favicon at a.com/icon.png and -then b.com is loaded and has the exact same favicon, the cache entry is -shared which permits b.com to notice that a.com was visited. The end -goal of this task is to prevent cross-origin cache leaks. - -This CL integrates the new variant of GetFaviconIDForFaviconURL into -UpdateFaviconMappingsAndFetch so that we filter the results per-pageurl -based on the origin. This should fully resolve the task, although the db -itself isn't fully partitioned. - -This CL is part of a series: -(1) Cache browser test -(2) Add new cache check function -(3) Stop cross-origin cache hits - -Bug: 1300214 -Change-Id: I9bf04982abea136a00e2b5252726a1cdef8e8550 -Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3822882 -Reviewed-by: Scott Violet -Auto-Submit: Ari Chivukula -Commit-Queue: Ari Chivukula -Cr-Commit-Position: refs/heads/main@{#1033798} - -License: GPL-3.0-only - https://spdx.org/licenses/GPL-3.0-only.html ---- - components/favicon/core/favicon_backend.cc | 1 + - 1 file changed, 1 insertion(+) - -diff --git a/components/favicon/core/favicon_backend.cc b/components/favicon/core/favicon_backend.cc ---- a/components/favicon/core/favicon_backend.cc -+++ b/components/favicon/core/favicon_backend.cc -@@ -808,4 +808,5 @@ bool FaviconBackend::ClearAllExcept(const std::vector& kept_page_urls) { - db_->BeginTransaction(); - return true; - } -+// DROP THIS PATCH - } // namespace favicon --- -2.25.1