Medium chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactIncorrect authorization in TabGroups
DescriptionIncorrect authorization in TabGroups
ComponentTabGroups
Bug ClassLogic Error
Tracker511822878
Fix commit4cc022983800 (chromium/src) +253/-20
CISA KEVNot listed
CreditedGoogle
Disclosed2026-08-25

Changed Functions

FunctionChangeNotes
for
components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cc
modified

Files Changed

  • chrome/browser/sync/test/integration/single_client_shared_tab_group_data_sync_test.cc
  • components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cc
  • components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.h
From 4cc022983800a4043f2c8d438dd09d12a7facc92 Mon Sep 17 00:00:00 2001
From: Rushan Suleymanov <[email protected]>
Date: Wed, 22 Jul 2026 05:19:38 -0700
Subject: [PATCH] Prevent cross-collaboration deletions in shared tab groups

In SharedTabGroupDataSyncBridge::DeleteDataFromLocalStorage, verify that
the collaboration ID of the incoming deletion entity matches the
collaboration ID of the target group, tab, or missing tab group.

If the collaboration IDs do not match, the deletion is ignored to
prevent cross-collaboration deletion of data across access boundaries.

Bug: 511822878
Change-Id: I6c0140356babb0bfcb527e3024c7194320e973e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8130940
Reviewed-by: David Pennington <[email protected]>
Commit-Queue: Rushan Suleymanov <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1666194}
---

diff --git a/chrome/browser/sync/test/integration/single_client_shared_tab_group_data_sync_test.cc b/chrome/browser/sync/test/integration/single_client_shared_tab_group_data_sync_test.cc
index b187c22a..8dcd1d47 100644
--- a/chrome/browser/sync/test/integration/single_client_shared_tab_group_data_sync_test.cc
+++ b/chrome/browser/sync/test/integration/single_client_shared_tab_group_data_sync_test.cc
@@ -780,6 +780,79 @@
   EXPECT_TRUE(SharedTabGroupDataErrorChecker(GetSyncService(0)).Wait());
 }
 
+IN_PROC_BROWSER_TEST_P(SingleClientSharedTabGroupDataSyncTest,
+                       ShouldIgnoreCrossCollaborationGroupDeletion) {
+  ASSERT_TRUE(SetupSync());
+
+  const base::Uuid kGroupGuid = base::Uuid::GenerateRandomV4();
+  const syncer::CollaborationId kCollaborationId1("collaboration_1");
+  const syncer::CollaborationId kCollaborationId2("collaboration_2");
+  RegisterCollaboration(kCollaborationId1);
+  RegisterCollaboration(kCollaborationId2);
+
+  const sync_pb::SharedTabGroupDataSpecifics shared_group_specifics =
+      MakeSharedTabGroupSpecifics(
+          kGroupGuid,
+          /*originating_saved_group_guid=*/base::Uuid::GenerateRandomV4(),
+          "title", sync_pb::SharedTabGroup_Color_CYAN);
+  AddSpecificsToFakeServer(shared_group_specifics, kCollaborationId1);
+
+  const sync_pb::SharedTabGroupDataSpecifics shared_tab_specifics =
+      MakeSharedTabGroupTabSpecifics(base::Uuid::GenerateRandomV4(), kGroupGuid,
+                                     "tab 1", GURL("http://google.com/1"));
+  AddSpecificsToFakeServer(shared_tab_specifics, kCollaborationId1);
+
+  ASSERT_TRUE(SavedTabOrGroupExistsChecker(GetTabGroupSyncService(), kGroupGuid)
+                  .Wait());
+  ASSERT_THAT(GetAllTabGroups(), SizeIs(1));
+
+  // Simulate an injected tombstone for the group from a different
+  // collaboration.
+  InjectTombstoneToFakeServer(shared_group_specifics, kCollaborationId2);
+
+  ASSERT_TRUE(AwaitQuiescence());
+
+  // Group and tab should remain intact.
+  ASSERT_THAT(GetAllTabGroups(), SizeIs(1));
+  EXPECT_THAT(GetAllTabGroups().front().saved_tabs(), SizeIs(1));
+}
+
+IN_PROC_BROWSER_TEST_P(SingleClientSharedTabGroupDataSyncTest,
+                       ShouldIgnoreCrossCollaborationTabDeletion) {
+  ASSERT_TRUE(SetupSync());
+
+  const base::Uuid kGroupGuid = base::Uuid::GenerateRandomV4();
+  const syncer::CollaborationId kCollaborationId1("collaboration_1");
+  const syncer::CollaborationId kCollaborationId2("collaboration_2");
+  RegisterCollaboration(kCollaborationId1);
+  RegisterCollaboration(kCollaborationId2);
+
+  const sync_pb::SharedTabGroupDataSpecifics shared_group_specifics =
+      MakeSharedTabGroupSpecifics(
+          kGroupGuid,
+          /*originating_saved_group_guid=*/base::Uuid::GenerateRandomV4(),
+          "title", sync_pb::SharedTabGroup_Color_CYAN);
+  AddSpecificsToFakeServer(shared_group_specifics, kCollaborationId1);
+
+  const sync_pb::SharedTabGroupDataSpecifics shared_tab_specifics =
+      MakeSharedTabGroupTabSpecifics(base::Uuid::GenerateRandomV4(), kGroupGuid,
+                                     "tab 1", GURL("http://google.com/1"));
+  AddSpecificsToFakeServer(shared_tab_specifics, kCollaborationId1);
+
+  ASSERT_TRUE(SavedTabOrGroupExistsChecker(GetTabGroupSyncService(), kGroupGuid)
+                  .Wait());
+  ASSERT_THAT(GetAllTabGroups(), SizeIs(1));
+
+  // Simulate an injected tombstone for the tab from a different collaboration.
+  InjectTombstoneToFakeServer(shared_tab_specifics, kCollaborationId2);
+
+  ASSERT_TRUE(AwaitQuiescence());
+
+  // Group and tab should remain intact.
+  ASSERT_THAT(GetAllTabGroups(), SizeIs(1));
+  EXPECT_THAT(GetAllTabGroups().front().saved_tabs(), SizeIs(1));
+}
+
 // Android doesn't support PRE_ tests.
 #if !BUILDFLAG(IS_ANDROID)
 IN_PROC_BROWSER_TEST_P(SingleClientSharedTabGroupDataSyncTest,
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 7ecdde05e..5f15f4e 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
@@ -644,14 +644,15 @@
 
   // Process group and tab deletions first.
   for (const std::unique_ptr<syncer::EntityChange>& change : delete_changes) {
-    GaiaId last_updated_by;
-    if (change->data().collaboration_metadata) {
-      last_updated_by =
-          change->data().collaboration_metadata->last_updated_by();
+    if (!change->data().collaboration_metadata.has_value()) {
+      // This should never happen. Skip deletion in this case for safety.
+      continue;
     }
-    DeleteDataFromLocalStorage(change->storage_key(),
-                               std::move(last_updated_by),
-                               *ongoing_write_batch_);
+    GaiaId last_updated_by =
+        change->data().collaboration_metadata->last_updated_by();
+    DeleteDataFromLocalStorage(
+        change->storage_key(), *change->data().collaboration_metadata,
+        std::move(last_updated_by), *ongoing_write_batch_);
   }
 
   // Sort tab updates and creations in the reversed order. This is required to
@@ -1361,10 +1362,9 @@
 
 void SharedTabGroupDataSyncBridge::DeleteDataFromLocalStorage(
     const std::string& storage_key,
+    const syncer::CollaborationMetadata& collaboration_metadata,
     GaiaId removed_by,
     syncer::DataTypeStore::WriteBatch& write_batch) {
-  write_batch.DeleteData(storage_key);
-
   base::Uuid guid = base::Uuid::ParseLowercase(storage_key);
   if (!guid.is_valid()) {
     return;
@@ -1372,7 +1372,13 @@
 
   // Check if the model contains the group guid. If so, remove that group and
   // all of its tabs.
-  if (model_wrapper_->GetGroup(guid)) {
+  if (const SavedTabGroup* group = model_wrapper_->GetGroup(guid)) {
+    if (group->collaboration_id() !=
+        collaboration_metadata.collaboration_id()) {
+      DVLOG(1) << "Ignoring deletion of group from a different collaboration";
+      return;
+    }
+    write_batch.DeleteData(storage_key);
     std::erase(tab_groups_waiting_for_commit_, guid);
     model_wrapper_->RemoveGroup(guid);
     return;
@@ -1380,9 +1386,31 @@
 
   if (const SavedTabGroup* group_containing_tab =
           model_wrapper_->GetGroupContainingTab(guid)) {
+    if (group_containing_tab->collaboration_id() !=
+        collaboration_metadata.collaboration_id()) {
+      DVLOG(1) << "Ignoring deletion of tab from a different collaboration";
+      return;
+    }
+    write_batch.DeleteData(storage_key);
     model_wrapper_->RemoveTabFromGroup(group_containing_tab->saved_guid(), guid,
                                        std::move(removed_by));
+    return;
   }
+
+  auto it = tabs_missing_groups_.find(guid);
+  if (it != tabs_missing_groups_.end()) {
+    if (it->second.collaboration_metadata.collaboration_id() !=
+        collaboration_metadata.collaboration_id()) {
+      DVLOG(1) << "Ignoring deletion of tab missing group from a different "
+                  "collaboration";
+      return;
+    }
+    write_batch.DeleteData(storage_key);
+    tabs_missing_groups_.erase(it);
+    return;
+  }
+
+  write_batch.DeleteData(storage_key);
 }
 
 void SharedTabGroupDataSyncBridge::SendToSync(
diff --git a/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.h b/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.h
index 0eb8bfd..e93b031 100644
--- a/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.h
+++ b/components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.h
@@ -149,6 +149,7 @@
   // tabs will be removed in addition to the group.
   void DeleteDataFromLocalStorage(
       const std::string& storage_key,
+      const syncer::CollaborationMetadata& collaboration_metadata,
       GaiaId removed_by,
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/chrome/browser/sync/test/integration/single_client_shared_tab_group_data_sync_test.cc b/chrome/browser/sync/test/integration/single_client_shared_tab_group_data_sync_test.cc
index b187c22a..8dcd1d47 100644
--- a/chrome/browser/sync/test/integration/single_client_shared_tab_group_data_sync_test.cc
+++ b/chrome/browser/sync/test/integration/single_client_shared_tab_group_data_sync_test.cc
@@ -780,6 +780,79 @@
   EXPECT_TRUE(SharedTabGroupDataErrorChecker(GetSyncService(0)).Wait());
 }
 
+IN_PROC_BROWSER_TEST_P(SingleClientSharedTabGroupDataSyncTest,
+                       ShouldIgnoreCrossCollaborationGroupDeletion) {
+  ASSERT_TRUE(SetupSync());
+
+  const base::Uuid kGroupGuid = base::Uuid::GenerateRandomV4();
+  const syncer::CollaborationId kCollaborationId1("collaboration_1");
+  const syncer::CollaborationId kCollaborationId2("collaboration_2");
+  RegisterCollaboration(kCollaborationId1);
+  RegisterCollaboration(kCollaborationId2);
+
+  const sync_pb::SharedTabGroupDataSpecifics shared_group_specifics =
+      MakeSharedTabGroupSpecifics(
+          kGroupGuid,
+          /*originating_saved_group_guid=*/base::Uuid::GenerateRandomV4(),
+          "title", sync_pb::SharedTabGroup_Color_CYAN);
+  AddSpecificsToFakeServer(shared_group_specifics, kCollaborationId1);
+
+  const sync_pb::SharedTabGroupDataSpecifics shared_tab_specifics =
+      MakeSharedTabGroupTabSpecifics(base::Uuid::GenerateRandomV4(), kGroupGuid,
+                                     "tab 1", GURL("http://google.com/1"));
+  AddSpecificsToFakeServer(shared_tab_specifics, kCollaborationId1);
+
+  ASSERT_TRUE(SavedTabOrGroupExistsChecker(GetTabGroupSyncService(), kGroupGuid)
+                  .Wait());
+  ASSERT_THAT(GetAllTabGroups(), SizeIs(1));
+
+  // Simulate an injected tombstone for the group from a different
+  // collaboration.
+  InjectTombstoneToFakeServer(shared_group_specifics, kCollaborationId2);
+
+  ASSERT_TRUE(AwaitQuiescence());
+
+  // Group and tab should remain intact.
+  ASSERT_THAT(GetAllTabGroups(), SizeIs(1));
+  EXPECT_THAT(GetAllTabGroups().front().saved_tabs(), SizeIs(1));
+}
+
+IN_PROC_BROWSER_TEST_P(SingleClientSharedTabGroupDataSyncTest,
+                       ShouldIgnoreCrossCollaborationTabDeletion) {
+  ASSERT_TRUE(SetupSync());
+
+  const base::Uuid kGroupGuid = base::Uuid::GenerateRandomV4();
+  const syncer::CollaborationId kCollaborationId1("collaboration_1");
+  const syncer::CollaborationId kCollaborationId2("collaboration_2");
+  RegisterCollaboration(kCollaborationId1);
+  RegisterCollaboration(kCollaborationId2);
+
+  const sync_pb::SharedTabGroupDataSpecifics shared_group_specifics =
+      MakeSharedTabGroupSpecifics(
+          kGroupGuid,
+          /*originating_saved_group_guid=*/base::Uuid::GenerateRandomV4(),
+          "title", sync_pb::SharedTabGroup_Color_CYAN);
+  AddSpecificsToFakeServer(shared_group_specifics, kCollaborationId1);
+
+  const sync_pb::SharedTabGroupDataSpecifics shared_tab_specifics =
+      MakeSharedTabGroupTabSpecifics(base::Uuid::GenerateRandomV4(), kGroupGuid,
+                                     "tab 1", GURL("http://google.com/1"));
+  AddSpecificsToFakeServer(shared_tab_specifics, kCollaborationId1);
+
+  ASSERT_TRUE(SavedTabOrGroupExistsChecker(GetTabGroupSyncService(), kGroupGuid)
+                  .Wait());
+  ASSERT_THAT(GetAllTabGroups(), SizeIs(1));
+
+  // Simulate an injected tombstone for the tab from a different collaboration.
+  InjectTombstoneToFakeServer(shared_tab_specifics, kCollaborationId2);
+
+  ASSERT_TRUE(AwaitQuiescence());
+
+  // Group and tab should remain intact.
+  ASSERT_THAT(GetAllTabGroups(), SizeIs(1));
+  EXPECT_THAT(GetAllTabGroups().front().saved_tabs(), SizeIs(1));
+}
+
 // Android doesn't support PRE_ tests.
 #if !BUILDFLAG(IS_ANDROID)
 IN_PROC_BROWSER_TEST_P(SingleClientSharedTabGroupDataSyncTest,
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 76542232..8737352d 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
@@ -374,8 +374,12 @@
 }
 
 std::unique_ptr<syncer::EntityChange> CreateDeleteEntityChange(
-    const std::string& storage_key) {
-  return syncer::EntityChange::CreateDelete(storage_key, syncer::EntityData());
+    const std::string& storage_key,
+    const CollaborationId& collaboration_id) {
+  return syncer::EntityChange::CreateDelete(
+      storage_key, CreateEntityData(sync_pb::SharedTabGroupDataSpecifics(),
+                                    collaboration_id, kDefaultGaiaId,
+                                    /*updated_by=*/kDefaultGaiaId));
 }
 
 std::vector<syncer::EntityData> ExtractEntityDataFromBatch(
@@ -900,8 +904,9 @@
           .SetCollaborationId(CollaborationId("collaboration 2")));
   ASSERT_EQ(model()->Count(), 2);
 
-  ApplySingleEntityChange(CreateDeleteEntityChange(
-      group_to_delete.saved_guid().AsLowercaseString()));
+  ApplySingleEntityChange(
+      CreateDeleteEntityChange(group_to_delete.saved_guid().AsLowercaseString(),
+                               CollaborationId("collaboration")));
 
   EXPECT_THAT(
       model()->saved_tab_groups(),
@@ -927,7 +932,8 @@
   ASSERT_THAT(model()->saved_tab_groups().front().saved_tabs(), SizeIs(2));
 
   ApplySingleEntityChange(CreateDeleteEntityChange(
-      tab_to_delete.saved_tab_guid().AsLowercaseString()));
+      tab_to_delete.saved_tab_guid().AsLowercaseString(),
+      CollaborationId("collaboration")));
 
   ASSERT_EQ(model()->Count(), 1);
   EXPECT_THAT(
@@ -1857,8 +1863,8 @@
   syncer::EntityChangeList change_list;
   change_list.push_back(
       CreateUpdateEntityChange(tab_1_specifics, kCollaborationId));
-  change_list.push_back(
-      CreateDeleteEntityChange(StorageKeyForTab(group.saved_tabs()[0])));
+  change_list.push_back(CreateDeleteEntityChange(
+      StorageKeyForTab(group.saved_tabs()[0]), kCollaborationId));
 
   bridge()->ApplyIncrementalSyncChanges(bridge()->CreateMetadataChangeList(),
                                         std::move(change_list));
@@ -1927,8 +1933,8 @@
   EXPECT_CALL(mock_processor(), Put).Times(0);
 
   syncer::EntityChangeList change_list;
-  change_list.push_back(
-      CreateDeleteEntityChange(StorageKeyForTab(group.saved_tabs()[0])));
+  change_list.push_back(CreateDeleteEntityChange(
+      StorageKeyForTab(group.saved_tabs()[0]), kCollaborationId));
   bridge()->ApplyIncrementalSyncChanges(bridge()->CreateMetadataChangeList(),
                                         std::move(change_list));
 
@@ -2265,7 +2271,8 @@
       MakeTabSpecifics("Tab 2", GURL("http://google.com/2"), group.saved_guid(),
                        GenerateRandomUniquePosition());
   syncer::EntityChangeList change_list;
-  change_list.push_back(CreateDeleteEntityChange(StorageKeyForTab(tab)));
+  change_list.push_back(
+      CreateDeleteEntityChange(StorageKeyForTab(tab), kCollaborationId));
   change_list.push_back(
       CreateUpdateEntityChange(new_tab_specifics, kCollaborationId));
   bridge()->ApplyIncrementalSyncChanges(bridge()->CreateMetadataChangeList(),
@@ -2451,6 +2458,130 @@
 }
 
 TEST_F(SharedTabGroupDataSyncBridgeTest,
+       ShouldIgnoreCrossCollaborationGroupDeletion) {
+  const CollaborationId kCollaborationId1("collaboration 1");
+  const CollaborationId kCollaborationId2("collaboration 2");
+  ASSERT_TRUE(InitializeBridgeAndModel());
+
+  // Add a group and a tab in collaboration 1.
+  sync_pb::SharedTabGroupDataSpecifics group_specifics =
+      MakeTabGroupSpecifics("title", sync_pb::SharedTabGroup::BLUE);
+  const base::Uuid group_id =
+      base::Uuid::ParseLowercase(group_specifics.guid());
+  sync_pb::SharedTabGroupDataSpecifics tab_specifics =
+      MakeTabSpecifics("tab title 1", GURL("https://google.com/1"), group_id,
+                       GenerateRandomUniquePosition());
+
+  syncer::EntityChangeList change_list;
+  change_list.push_back(
+      CreateAddEntityChange(group_specifics, kCollaborationId1));
+  change_list.push_back(
+      CreateAddEntityChange(tab_specifics, kCollaborationId1));
+  bridge()->ApplyIncrementalSyncChanges(bridge()->CreateMetadataChangeList(),
+                                        std::move(change_list));
+
+  ASSERT_THAT(model()->saved_tab_groups(), SizeIs(1));
+  ASSERT_THAT(model()->saved_tab_groups().front().saved_tabs(), SizeIs(1));
+
+  // Try to delete the group from collaboration 2 (cross-collaboration
+  // deletion).
+  syncer::EntityChangeList delete_group_list;
+  delete_group_list.push_back(syncer::EntityChange::CreateDelete(
+      group_specifics.guid(),
+      CreateEntityData(group_specifics, kCollaborationId2, kDefaultGaiaId,
+                       /*updated_by=*/kDefaultGaiaId)));
+  EXPECT_FALSE(
+      bridge()
+          ->ApplyIncrementalSyncChanges(bridge()->CreateMetadataChangeList(),
+                                        std::move(delete_group_list))
+          .has_value());
+
+  // Group and tab should remain intact.
+  ASSERT_THAT(model()->saved_tab_groups(), SizeIs(1));
+  ASSERT_THAT(model()->saved_tab_groups().front().saved_tabs(), SizeIs(1));
+}
+
+TEST_F(SharedTabGroupDataSyncBridgeTest,
+       ShouldIgnoreCrossCollaborationTabDeletion) {
+  const CollaborationId kCollaborationId1("collaboration 1");
+  const CollaborationId kCollaborationId2("collaboration 2");
+  ASSERT_TRUE(InitializeBridgeAndModel());
+
+  // Add a group and a tab in collaboration 1.
+  sync_pb::SharedTabGroupDataSpecifics group_specifics =
+      MakeTabGroupSpecifics("title", sync_pb::SharedTabGroup::BLUE);
+  const base::Uuid group_id =
+      base::Uuid::ParseLowercase(group_specifics.guid());
+  sync_pb::SharedTabGroupDataSpecifics tab_specifics =
+      MakeTabSpecifics("tab title 1", GURL("https://google.com/1"), group_id,
+                       GenerateRandomUniquePosition());
+
+  syncer::EntityChangeList change_list;
+  change_list.push_back(
+      CreateAddEntityChange(group_specifics, kCollaborationId1));
+  change_list.push_back(
+      CreateAddEntityChange(tab_specifics, kCollaborationId1));
+  bridge()->ApplyIncrementalSyncChanges(bridge()->CreateMetadataChangeList(),
+                                        std::move(change_list));
+
+  ASSERT_THAT(model()->saved_tab_groups(), SizeIs(1));
+  ASSERT_THAT(model()->saved_tab_groups().front().saved_tabs(), SizeIs(1));
+
+  // Try to delete the tab from collaboration 2 (cross-collaboration deletion).
+  syncer::EntityChangeList delete_tab_list;
+  delete_tab_list.push_back(syncer::EntityChange::CreateDelete(
+      tab_specifics.guid(),
+      CreateEntityData(tab_specifics, kCollaborationId2, kDefaultGaiaId,
+                       /*updated_by=*/kDefaultGaiaId)));
+  EXPECT_FALSE(
+      bridge()
+          ->ApplyIncrementalSyncChanges(bridge()->CreateMetadataChangeList(),
+                                        std::move(delete_tab_list))
+          .has_value());
+
+  // Group and tab should still remain intact.
+  ASSERT_THAT(model()->saved_tab_groups(), SizeIs(1));
+  ASSERT_THAT(model()->saved_tab_groups().front().saved_tabs(), SizeIs(1));
+}
+
+TEST_F(SharedTabGroupDataSyncBridgeTest,
+       ShouldIgnoreCrossCollaborationTabMissingGroupDeletion) {
+  const CollaborationId kCollaborationId1("collaboration 1");
+  const CollaborationId kCollaborationId2("collaboration 2");
+  const base::Uuid kMissingGroupGuid = base::Uuid::GenerateRandomV4();
+  ASSERT_TRUE(InitializeBridgeAndModel());
+
+  // Add a tab missing group in collaboration 1.
+  sync_pb::SharedTabGroupDataSpecifics tab_specifics =
+      MakeTabSpecifics("tab title", GURL("http://google.com/1"),
+                       kMissingGroupGuid, GenerateRandomUniquePosition());
+
+  ApplySingleEntityChange(
+      CreateAddEntityChange(tab_specifics, kCollaborationId1));
+
+  // Verify that the tab missing group is present.
+  ASSERT_THAT(ExtractEntityDataFromBatch(bridge()->GetAllDataForDebugging()),
+              SizeIs(1));
+
+  // Try to delete the tab missing group from collaboration 2
+  // (cross-collaboration deletion).
+  syncer::EntityChangeList delete_tab_list;
+  delete_tab_list.push_back(syncer::EntityChange::CreateDelete(
+      tab_specifics.guid(),
+      CreateEntityData(tab_specifics, kCollaborationId2, kDefaultGaiaId,
+                       /*updated_by=*/kDefaultGaiaId)));
+  EXPECT_FALSE(
+      bridge()
+          ->ApplyIncrementalSyncChanges(bridge()->CreateMetadataChangeList(),
+                                        std::move(delete_tab_list))
+          .has_value());
+
+  // The tab missing group should still remain intact.
+  EXPECT_THAT(ExtractEntityDataFromBatch(bridge()->GetAllDataForDebugging()),
+              SizeIs(1));
+}
+
+TEST_F(SharedTabGroupDataSyncBridgeTest,
        ShouldResolveTabsMissingGroupsOnRemoteUpdate) {
   const CollaborationId kCollaborationId("collaboration");
   const base::Uuid kMissingGroupGuid = base::Uuid::GenerateRandomV4();
Loading diff…

Original Bug Report

reported by [email protected]

Cross-collaboration shared tab group deletion via storage key collision

Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the Chrome Security team. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.

Overview: A potential logic flaw in SharedTabGroupDataSyncBridge allows an attacker in one collaboration to delete a victim’s shared tab group in a different collaboration. This occurs because the local StorageKey namespace uses only the group GUID, bypassing a production-disabled DCHECK during sync, leading to unauthorized data overwrite and deletion.

Affected files:

  • components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cc
  • components/saved_tab_groups/internal/sync_bridge_tab_group_model_wrapper.cc
  • components/sync/model/processor_entity_tracker.cc

Estimated timestamp from git blame: 2025-06-02

Summary

SharedTabGroupDataSyncBridge maps remote sync entities to local storage. It correctly generates a globally unique ClientTag for each entity using guid + '|' + collaboration_id. However, the local database StorageKey uses only the guid. Because the StorageKey lacks the collaboration ID, an attacker can create a local storage key collision by injecting an entity into an unrelated collaboration using the GUID of a victim’s group.

Potential Attack Scenario

Based on source code analysis, the following sequence of events outlines how an attacker could exploit this vulnerability. Note that these are potential steps, as our analysis environment cannot execute live Proof of Concepts.

Prerequisites:

  • The victim is a member of Collaboration 1 (C1) and Collaboration 2 (C2).
  • The attacker is a member of Collaboration 1 (C1).
  • The attacker knows the 128-bit UUID (G2) of a shared tab group belonging to the victim in C2 (e.g., from prior membership or a sharing link).

Step-by-Step Exploitation:

  1. Injection: The attacker commits a crafted SHARED_TAB_GROUP_DATA tab entity to C1 using the victim’s group GUID (G2). To bypass collaboration ID checks later, the attacker sets the tab’s parent group GUID to a dummy, non-existent value.
  2. DCHECK Bypass and Collision: The victim receives the sync update. The sync processor extracts the StorageKey (G2). In ProcessorEntityTracker::AddInternal, a check verifies that the storage key is unique (DCHECK(storage_key_to_tag_hash_.find(storage_key) == storage_key_to_tag_hash_.end())). Because this is a DCHECK, it compiles out in production, allowing the local storage_key_to_tag_hash_ mapping for G2 to be silently overwritten, pointing to the attacker’s C1 entity.
  3. Database Overwrite: The bridge processes the tab update (ApplyRemoteTabUpdate). Because the dummy parent group is missing, the code falls into a branch that defers processing and explicitly bypasses collaboration ID verification. It calls StoreSharedTab, which writes the attacker’s tab data directly into the DataTypeStore using the G2 storage key, silently overwriting the victim’s stored database record for the C2 group.
  4. Cross-Collaboration Deletion: The attacker tombstones (deletes) the crafted C1 entity. The victim receives the deletion action for the G2 storage key. DeleteDataFromLocalStorage deletes the database record and blindly removes the live C2 group from the in-memory SavedTabGroupModel without verifying the collaboration ID.

Impact

  • Denial of Service / Data Loss: An attacker can remotely close all tabs associated with a victim’s shared tab group and irreversibly delete the group from their profile.
  • Information Disclosure (Potential): If the victim edits their C2 group after Step 3 but before Step 4, ClientTagBasedDataTypeProcessor::Put may use the corrupted tracking map to sync the victim’s private C2 changes back to the attacker’s C1 collaboration.

Proposed Fix

  1. Unique Storage Keys: Update SharedTabGroupDataSyncBridge::GetStorageKey to include the collaboration ID (e.g., guid + '|' + collaboration_id) to ensure the flat local database namespace is unique across collaborations.
  2. Harden Collision Checks: Promote the DCHECK in ProcessorEntityTracker::AddInternal (and related methods) to a CHECK to prevent silent state corruption in production builds.
  3. Strict Verification: Ensure that ApplyRemoteTabUpdate validates the collaboration_id even when the parent group is missing, before persisting data to the DataTypeStore.

Evaluated with Chrome root at commit: eca8648a4e1cdfdda68c495a6003059fed641955


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. And please feel free to reach out to me directly if you have concerns or feedback on the project.

View on issue tracker