v106 drop patch already upstream
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -1,157 +0,0 @@
|
||||
From: Ari Chivukula <arichiv@chromium.org>
|
||||
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 <sky@chromium.org>
|
||||
Auto-Submit: Ari Chivukula <arichiv@chromium.org>
|
||||
Commit-Queue: Ari Chivukula <arichiv@chromium.org>
|
||||
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<base::RefCountedStaticMemory> 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
|
||||
@@ -1,43 +0,0 @@
|
||||
From: Ari Chivukula <arichiv@chromium.org>
|
||||
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 <sky@chromium.org>
|
||||
Auto-Submit: Ari Chivukula <arichiv@chromium.org>
|
||||
Commit-Queue: Ari Chivukula <arichiv@chromium.org>
|
||||
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<GURL>& kept_page_urls) {
|
||||
db_->BeginTransaction();
|
||||
return true;
|
||||
}
|
||||
+// DROP THIS PATCH
|
||||
} // namespace favicon
|
||||
--
|
||||
2.25.1
|
||||
Reference in New Issue
Block a user