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 <rushans@google.com>
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 <rushans@google.com>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
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.