Low watch CVSS 5.4 chrome Input Validation 📄 Reporter bug report 🔧 Commit mapped

Overview

Low
Severity
5.4
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInsufficient validation of untrusted input in TabGroups
DescriptionInsufficient validation of untrusted input in TabGroups
ComponentTabGroups
Bug ClassInput Validation
Action tierwatch — logic/policy bug — security-relevant, no memory-unsafety
Computed by PatchGap's deterministic exploitability ruler from the fixing commit. patch-now = RCE-class & attacker-reachable in a default config, or exploited in the wild; watch = real security signal below that bar; noise = no security relevance.
Tracker495985532
Fix commit4a763a35404a (chromium/src) +21/-16
CWECWE-20 (Improper input validation)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:L/I:N/A:L
CISA KEVNot listed
CreditedGoogle
Reported2026-03-25 (per vendor advisory)
Fix landed2026-03-31 (public git)
Disclosed2026-05-05
Patch gap35 days — fix public in source before the advisory

Background

Shared tab group
A tab group synced and shared across users in a collaboration, backed by SharedTabGroupDataSpecifics sync protocol messages.
Collaboration ID
A stable identifier binding a shared group to a specific collaboration; the invariant is that a shared group never changes collaboration IDs.
`MergeRemoteGroupMetadata`
A model_wrapper_ call that applies remote fields (title, color, timestamps, last-updater) onto an existing local SavedTabGroup.
`CollaborationMetadata`
Metadata attached to an incoming sync update, including the collaboration_id and last_updated_by of the remote change.

Root Cause Analysis

In the sync-bridge method that applies a remote update to an existing shared group (its enclosing name is not shown in the @@ header), the code performed MergeRemoteGroupMetadata — actually writing the remote title, color, and timestamp into the local group — and only afterwards compared existing_group->collaboration_id() against collaboration_metadata.collaboration_id(). The mutation therefore happened before validation, so an update carrying an unexpected/mismatched collaboration ID would still mutate the group’s state even though the subsequent mismatch check returned std::nullopt; the violated invariant is that updates must be authorized by a matching collaboration ID before any data is applied. A TODO(crbug.com/381540386) in the removed code explicitly noted the check should be moved before the merge.

The fix reorders the logic: it first fetches existing_group via GetGroup, performs the collaboration_id() mismatch check (reporting kSharedTabGroupUnexpectedCollaborationIdForGroup and returning early), and only then calls MergeRemoteGroupMetadata, so a mismatched update is rejected without altering local state — as the regression test asserts the group’s color and the tab’s title remain unchanged.

Key insight
The collaboration-ID authorization check ran after the remote data had already been merged into the local group; the fix moves the check before MergeRemoteGroupMetadata so unauthorized updates are rejected without mutating group state.

Attack Path

  1. Target an existing shared group The victim has a local shared tab group with a known saved_guid and a legitimate collaboration ID.
  2. Craft a mismatched update An attacker with sync write access sends a SharedTabGroupDataSpecifics update for that guid carrying an unexpected collaboration_id and attacker-chosen title/color/tab fields.
  3. Pre-patch merge-before-check The bridge calls MergeRemoteGroupMetadata, writing the attacker’s fields into the local group, before the collaboration-ID mismatch is detected.
  4. Unauthorized mutation persists Although the method returns std::nullopt, the local group (and, in the analogous tab path, tab title/URL) has already been altered by data that failed authorization.

Impact Assessment

An attacker able to deliver crafted sync updates could modify a shared tab group’s metadata (title, color) and tab contents despite supplying a collaboration ID that should have rejected the change, because the authorization check ran only after the merge. The precondition is the ability to inject SharedTabGroupDataSpecifics updates for a known group GUID through the sync channel; impact is data integrity/spoofing of shared tab group state, not memory corruption. The vendor advisory rates this low severity and classifies it as insufficient validation of untrusted input in TabGroups; no exploitation-in-the-wild is stated.

Changed Functions

FunctionChangeNotes
if
components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge.cc
modified
TEST_F
components/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.cc
  • components/saved_tab_groups/internal/shared_tab_group_data_sync_bridge_unittest.cc

Audit Directions

  • Validate-then-mutate ordering
    Audit sync/IPC handlers that apply remote data to local state, confirming every authorization/identity check runs before the mutating call rather than after.
  • Rejected-but-applied updates
    Flag code paths that return an error/nullopt yet have already written to shared model state earlier in the function.
  • Stale "move this check" TODOs
    Grep for TODOs noting a check should be moved before an operation; they often mark exactly this ordering class of vulnerability.
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) {
Loading diff…

Regression Test / PoC

shipped with the fix
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) {
Loading diff…

Original Bug Report

reported by [email protected]

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.cc
  • components/saved_tab_groups/internal/tab_group_sync_service_impl.cc
  • components/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:

  1. It looks up the existing group by GUID and calls model_wrapper_->MergeRemoteGroupMetadata(...).
  2. This immediately mutates the in-memory model (title, color, attribution) and notifies UI observers, spoofing the victim’s tab group UI.
  3. Finally, it checks if existing_group->collaboration_id() != collaboration_metadata.collaboration_id(). Since the IDs do not match, it returns a syncer::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

  1. The attacker and the victim are both members of Collaboration A.
  2. 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/guesses G.
  3. The attacker sends a SharedTabGroupDataSpecifics sync update to Collaboration A’s sync channel with guid=G, collaboration_id=A, and malicious metadata (e.g., spoofed title, color).
  4. The victim’s browser receives the update. ProcessorEntityTracker::AddInternal creates a new entity and maps G to the new update’s client_tag_hash, bypassing the duplicate storage_key check in release builds.
  5. SharedTabGroupDataSyncBridge::AddGroupToLocalStorage processes the update. It merges the malicious metadata into the in-memory group G before verifying the collaboration mismatch.
  6. The victim’s UI immediately reflects the spoofed metadata.
  7. 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.

View on issue tracker