CVE-2026-8003
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifcomponents/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cc |
modified | |
TEST_Fcomponents/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc |
modified |
Files Changed
components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cccomponents/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc
Patch
From 4a763a35404a170b648c7f3c5041b4e1ea86100a Mon Sep 17 00:00:00 2001 From: Rushan Suleymanov <[email protected]> Date: Tue, 31 Mar 2026 06:24:07 -0700 Subject: [PATCH] Validate collaboration ID before applying changes Bug: 495985532 Change-Id: Ifb04139cbb68c6a205edce3065289da0e226b268 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7711295 Commit-Queue: Rushan Suleymanov <[email protected]> Reviewed-by: Shakti Sahu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1607816} --- diff --git a/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cc b/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cc index 64d6c65..76d0611 100644 --- a/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cc +++ b/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cc @@ -1229,7 +1229,8 @@ CHECK(specifics.has_tab_group()); - if (!model_wrapper_->GetGroup(group_guid)) { + const SavedTabGroup* existing_group = model_wrapper_->GetGroup(group_guid); + if (!existing_group) { // This is a new remotely created group. Add the group from sync into local // storage. Note that on some platforms new remote groups may open in the // tab strip, and associate its local group ID. This is currently prevented @@ -1243,20 +1244,6 @@ return std::nullopt; } - // Update the existing group with remote data. - const SavedTabGroup* existing_group = - model_wrapper_->MergeRemoteGroupMetadata( - group_guid, base::UTF8ToUTF16(specifics.tab_group().title()), - SyncColorToTabGroupColor(specifics.tab_group().color()), - /*position=*/std::nullopt, - /*creator_cache_guid=*/std::nullopt, - /*last_updater_cache_guid=*/std::nullopt, - TimeFromWindowsEpochMicros( - specifics.update_time_windows_epoch_micros()), - collaboration_metadata.last_updated_by()); - CHECK(existing_group); - - // TODO(crbug.com/381540386): move this check before the merge. if (existing_group->collaboration_id() != collaboration_metadata.collaboration_id()) { // Shared tab groups should never change collaboration IDs. @@ -1265,6 +1252,17 @@ kSharedTabGroupUnexpectedCollaborationIdForGroup); } + // Update the existing group with remote data. + existing_group = model_wrapper_->MergeRemoteGroupMetadata( + group_guid, base::UTF8ToUTF16(specifics.tab_group().title()), + SyncColorToTabGroupColor(specifics.tab_group().color()), + /*position=*/std::nullopt, + /*creator_cache_guid=*/std::nullopt, + /*last_updater_cache_guid=*/std::nullopt, + TimeFromWindowsEpochMicros(specifics.update_time_windows_epoch_micros()), + collaboration_metadata.last_updated_by()); + CHECK(existing_group); + // Create new specifics in case some fields were merged. sync_pb::SharedTabGroupDataSpecifics updated_specifics = SharedTabGroupToSpecifics(*existing_group); diff --git a/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc b/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc index 1799b62f..34a7ed0 100644 --- a/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc +++ b/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc @@ -2003,6 +2003,8 @@ "http://google.com/1", u"tab", group.saved_guid(), /*position=*/0)); model()->AddedLocally(group); + // Update the group with an unexpected collaboration ID, the group should not + // be updated. sync_pb::SharedTabGroupDataSpecifics group_update_specifics = MakeTabGroupSpecifics("title", sync_pb::SharedTabGroup::BLUE); group_update_specifics.set_guid(group.saved_guid().AsLowercaseString()); @@ -2010,9 +2012,13 @@ group_update_specifics, CollaborationId("unexpected_collaboration_id"))), std::nullopt); + EXPECT_EQ(model()->Get(group.saved_guid())->color(), + tab_groups::TabGroupColorId::kGrey); + // Update the tab with an unexpected collaboration ID, the tab should not be + // updated. sync_pb::SharedTabGroupDataSpecifics tab_update_specifics = MakeTabSpecifics( - "tab", GURL("http://google.com/1"), + "new tab title", GURL("http://google.com/new"), /*group_id=*/group.saved_guid(), GenerateRandomUniquePosition()); tab_update_specifics.set_guid( group.saved_tabs()[0].saved_tab_guid().AsLowercaseString()); @@ -2020,6 +2026,7 @@ tab_update_specifics, CollaborationId("unexpected_collaboration_id"))), std::nullopt); + EXPECT_EQ(model()->Get(group.saved_guid())->saved_tabs()[0].title(), u"tab"); } TEST_F(SharedTabGroupDataSyncBridgeTest, ShouldStoreLocalIdOnRemoteUpdate) {
Regression Test / PoC
diff --git a/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc b/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc
index 1799b62f..34a7ed0 100644
--- a/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc
+++ b/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc
@@ -2003,6 +2003,8 @@
"http://google.com/1", u"tab", group.saved_guid(), /*position=*/0));
model()->AddedLocally(group);
+ // Update the group with an unexpected collaboration ID, the group should not
+ // be updated.
sync_pb::SharedTabGroupDataSpecifics group_update_specifics =
MakeTabGroupSpecifics("title", sync_pb::SharedTabGroup::BLUE);
group_update_specifics.set_guid(group.saved_guid().AsLowercaseString());
@@ -2010,9 +2012,13 @@
group_update_specifics,
CollaborationId("unexpected_collaboration_id"))),
std::nullopt);
+ EXPECT_EQ(model()->Get(group.saved_guid())->color(),
+ tab_groups::TabGroupColorId::kGrey);
+ // Update the tab with an unexpected collaboration ID, the tab should not be
+ // updated.
sync_pb::SharedTabGroupDataSpecifics tab_update_specifics = MakeTabSpecifics(
- "tab", GURL("http://google.com/1"),
+ "new tab title", GURL("http://google.com/new"),
/*group_id=*/group.saved_guid(), GenerateRandomUniquePosition());
tab_update_specifics.set_guid(
group.saved_tabs()[0].saved_tab_guid().AsLowercaseString());
@@ -2020,6 +2026,7 @@
tab_update_specifics,
CollaborationId("unexpected_collaboration_id"))),
std::nullopt);
+ EXPECT_EQ(model()->Get(group.saved_guid())->saved_tabs()[0].title(), u"tab");
}
TEST_F(SharedTabGroupDataSyncBridgeTest, ShouldStoreLocalIdOnRemoteUpdate) {
Original Bug Report
Potential UI Spoofing and Persistent DoS in SharedTabGroupDataSyncBridge
Project Fortify, an experimental security project, has identified the following potential security issue.
Overview: A logic flaw in SharedTabGroupDataSyncBridge allows an attacker to inject malicious metadata into a shared tab group belonging to a different collaboration. Because the in-memory tab group metadata is merged before verifying the incoming sync update’s collaboration ID, the UI is spoofed with attacker-controlled data. Furthermore, the subsequent validation failure triggers a persistent ModelError that permanently disables shared tab group sync for the victim.
Affected files:
components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cccomponents/saved_tab_groups/internal/tab_group_sync_service_impl.cccomponents/sync/model/processor_entity_tracker.cc
Estimated timestamp from git blame: 2025-07-07
Description
There is a potential vulnerability in SharedTabGroupDataSyncBridge::AddGroupToLocalStorage where it mutates the in-memory SavedTabGroupModel before validating that the incoming entity’s collaboration_id matches the existing group’s collaboration_id.
This vulnerability can be triggered by an attacker who shares one collaboration (Collaboration A) with a victim, and knows the GUID of a tab group in another collaboration (Collaboration B) that the victim belongs to.
When a remote sync update is received, the sync processor ProcessorEntityTracker::AddInternal relies on a DCHECK to enforce that different client_tag_hashes do not share the same storage_key. In release builds, this DCHECK is compiled out, allowing a malicious update from Collaboration A to overwrite the storage_key_to_tag_hash_ mapping of the victim’s group in Collaboration B (because SharedTabGroupDataSyncBridge::GetStorageKey only uses the GUID).
When the update reaches SharedTabGroupDataSyncBridge::AddGroupToLocalStorage:
- It looks up the existing group by GUID and calls
model_wrapper_->MergeRemoteGroupMetadata(...). - This immediately mutates the in-memory model (title, color, attribution) and notifies UI observers, spoofing the victim’s tab group UI.
- Finally, it checks if
existing_group->collaboration_id() != collaboration_metadata.collaboration_id(). Since the IDs do not match, it returns asyncer::ModelError(kSharedTabGroupUnexpectedCollaborationIdForGroup).
While the returned ModelError causes the database write batch to be discarded (so malicious changes are not persisted to disk), the in-memory model changes are NOT rolled back, leaving the UI spoofed. Moreover, the ModelError causes the SHARED_TAB_GROUP_DATA sync bridge to disconnect and drop pending updates. Because the malicious update is never acknowledged to the server, it remains pending. Upon every browser restart, the client will re-download the update and hit the identical error, resulting in a persistent Denial of Service (DoS) for all legitimate shared tab group synchronization.
(Note: This analysis is based on code review by an LLM agent and has not been verified with a live exploit.)
Potential Attack Scenario
- The attacker and the victim are both members of Collaboration A.
- The victim is also a member of Collaboration B, which contains a shared tab group with GUID
G. The attacker is not in B, but learns/guessesG. - The attacker sends a
SharedTabGroupDataSpecificssync update to Collaboration A’s sync channel withguid=G,collaboration_id=A, and malicious metadata (e.g., spoofed title, color). - The victim’s browser receives the update.
ProcessorEntityTracker::AddInternalcreates a new entity and mapsGto the new update’sclient_tag_hash, bypassing the duplicatestorage_keycheck in release builds. SharedTabGroupDataSyncBridge::AddGroupToLocalStorageprocesses the update. It merges the malicious metadata into the in-memory groupGbefore verifying the collaboration mismatch.- The victim’s UI immediately reflects the spoofed metadata.
- The bridge then detects the mismatch and returns a
ModelError, tearing down the sync connection and permanently breaking shared tab group sync for the victim until the malicious entity is removed.
Suggested Fix
In components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cc inside AddGroupToLocalStorage, the validation check for collaboration_id must be performed before calling model_wrapper_->MergeRemoteGroupMetadata(...).
// Update the existing group with remote data.
const SavedTabGroup* existing_group_for_check = model_wrapper_->GetGroup(group_guid);
if (existing_group_for_check->collaboration_id() != collaboration_metadata.collaboration_id()) {
// Shared tab groups should never change collaboration IDs.
return syncer::ModelError(
FROM_HERE, syncer::ModelError::Type::kSharedTabGroupUnexpectedCollaborationIdForGroup);
}
const SavedTabGroup* existing_group = model_wrapper_->MergeRemoteGroupMetadata(...);
Additionally, consider hardening ProcessorEntityTracker::AddInternal to explicitly reject or gracefully handle duplicate storage_key collisions in release builds, rather than relying solely on a DCHECK.
Evaluated with Chrome root at commit: 0eb4855bda702feaaa8b899336664f97e3df88b8
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. Please feel free to reach out to me if you have concerns or feedback.