Medium watch CVSS 5.9 chrome Input Validation 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
5.9
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactImproper input validation in Sync
DescriptionImproper input validation in Sync
ComponentSync
Sub-componentSync
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.
Tracker513737209
Fix commitdcebabc243ac (chromium/src) +45/-18
CWECWE-20 (Improper input validation)
CVSS vectorCVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N
CISA KEVNot listed
CreditedGoogle
Reported2026-05-16 (per vendor advisory)
Fix landed2026-07-22 (public git)
Disclosed2026-08-25
Patch gap34 days — fix public in source before the advisory

Background

`MIGRATION_DONE`
a sync-protocol error the server can send in a ClientToServerResponse, listing data types (migrated_data_type_id) whose server-side data was migrated. The client purges local state for those types and downloads them again.
`GetTypesToMigrate()`
a helper in components/sync/engine/syncer_proto_util.cc that turns the server-supplied migrated_data_type_id field numbers into the DataTypeSet the client will migrate.
`NIGORI`
the special sync data type that carries the account’s encryption state (keystore keys, custom-passphrase metadata, encrypted-types set), which protects all other encrypted sync data.
GC directive path (`UseGcDirective()`)
the parallel migration mechanism exercised by the integration tests. The patch leaves it unchanged and the NIGORI tests still run under it.

Root Cause Analysis

Before the patch, GetTypesToMigrate() returned whatever valid data types the server listed in migrated_data_type_id, with no exclusion, so a MIGRATION_DONE response could name NIGORI. The client would then purge and re-download its encryption-state type through the generic migration path. The commit says this is not supported “without questionable transitions”: handling a server-driven NIGORI reset safely needs special logic the migrator does not have. The broken rule is that untrusted server input should not be able to force a reset of the client’s local encryption state through a generic data-type mechanism.

The fix tightens the input check by calling types.Remove(NIGORI) on the parsed set before returning it, so a MIGRATION_DONE naming NIGORI is ignored for that type and still honoured for the others. The unit test confirms that an input of {BOOKMARKS, HISTORY_DELETE_DIRECTIVES, NIGORI, -1} now yields {BOOKMARKS, HISTORY_DELETE_DIRECTIVES}.

Key insight
The client trusted the server’s MIGRATION_DONE type list without exception, so it would treat its encryption-state type NIGORI like any other type and reset it. The fix removes NIGORI from the set in GetTypesToMigrate().

Attack Path

  1. Control sync server responses The attacker must be able to shape ClientToServerResponse messages, for example through a malicious or compromised sync server, or a custom sync endpoint set by policy or command-line. Ordinary web content does not reach this path (inferred from the protocol position).
  2. Send `MIGRATION_DONE` naming NIGORI The response has the MIGRATION_DONE error, with migrated_data_type_id including the specifics field number for NIGORI.
  3. Client schedules NIGORI migration Before the fix, GetTypesToMigrate() returned a set containing NIGORI, and the client purged its local encryption state and downloaded it again.
  4. Questionable encryption-state transition The re-downloaded NIGORI state comes from the server. This could replace or weaken the client’s encryption configuration, for example dropping custom-passphrase state (inferred; the patch shows no specific transition).

Impact Assessment

An attacker who controls sync server responses could make Chrome reset its NIGORI encryption state through the generic migration path, which the advisory classes as improper input validation in Sync. The precondition is control over the sync protocol stream, not web content. Whether this leads to downgraded encryption or exposure of synced data is (inferred), since the patch shows only that the transition was disallowed, and the advisory states no in-the-wild exploitation.

Changed Functions

FunctionChangeNotes
for
chrome/browser/sync/test/integration/migration_test.cc
modified
IN_PROC_BROWSER_TEST_P
chrome/browser/sync/test/integration/migration_test.cc
modified
TEST
components/sync/engine/syncer_proto_util_unittest.cc
modified

Files Changed

  • components/sync/engine/syncer_proto_util_unittest.cc
  • components/sync/engine/syncer_proto_util.cc
  • chrome/browser/sync/test/integration/migration_test.cc

Audit Directions

  • Server-driven reset of security-critical types
    Review other server-originated directives (GC directives such as the UseGcDirective() path exercised in the tests, CLEAR_SERVER_DATA, ENCRYPTION_OBSOLETE, throttling/disable responses) for whether they can reset or replace NIGORI or passphrase state without dedicated handling.
  • Allow-lists for type sets parsed from protobufs
    Where field numbers become DataTypeSets (e.g. GetDataTypeSetFromSpecificsFieldNumberList), check that control types (NIGORI, device info, sharing types) are filtered according to the operation, not only checked for validity.
  • Encryption-state downgrade transitions
    Audit the Nigori sync bridge for any path where state received from the server can lower the passphrase type or replace keys while a custom passphrase is active.
From dcebabc243ac3fc05519fe8d52fea65ac2490c3b Mon Sep 17 00:00:00 2001
From: Mikel Astiz <mastiz@chromium.org>
Date: Wed, 22 Jul 2026 09:29:42 -0700
Subject: [PATCH] [sync] Disallow MIGRATION_DONE for NIGORI

Supporting this properly for NIGORI without questionable transitions is
complex, so this patch simply excludes NIGORI from the datatypes that
can react to MIGRATION_DONE.

Change-Id: Ic4f48412d90abf4d97e2d6c7cd1fea40289d0c41
Fixed: 513737209
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8126459
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1666394}
---
diff --git a/components/sync/engine/syncer_proto_util.cc b/components/sync/engine/syncer_proto_util.cc
index 5442290..3fa45a0 100644
--- a/components/sync/engine/syncer_proto_util.cc
+++ b/components/sync/engine/syncer_proto_util.cc
@@ -238,8 +238,10 @@
 }  // namespace
 
 DataTypeSet GetTypesToMigrate(const ClientToServerResponse& response) {
-  return GetDataTypeSetFromSpecificsFieldNumberList(
+  DataTypeSet types = GetDataTypeSetFromSpecificsFieldNumberList(
       response.migrated_data_type_id());
+  types.Remove(NIGORI);
+  return types;
 }
 
 SyncProtocolError ConvertErrorPBToSyncProtocolError(
diff --git a/chrome/browser/sync/test/integration/migration_test.cc b/chrome/browser/sync/test/integration/migration_test.cc
index 84f431a..e672598 100644
--- a/chrome/browser/sync/test/integration/migration_test.cc
+++ b/chrome/browser/sync/test/integration/migration_test.cc
@@ -212,7 +212,7 @@
 
   enum TriggerMethod { MODIFY_PREF, MODIFY_BOOKMARK, TRIGGER_REFRESH };
 
-  syncer::DataTypeSet GetPreferredDataTypes() {
+  syncer::DataTypeSet GetPreferredDataTypesEligibleForMigration() {
     // SyncServiceImpl must already have been created before we can call
     // GetPreferredDataTypes().
     DCHECK(GetSyncService(0));
@@ -246,15 +246,21 @@
     // Doesn't make sense to migrate commit only types.
     preferred_data_types.RemoveAll(syncer::CommitOnlyTypes());
 
+    if (!UseGcDirective()) {
+      // NIGORI migration is disallowed for MIGRATION_DONE response.
+      preferred_data_types.Remove(syncer::NIGORI);
+    }
+
     return preferred_data_types;
   }
 
   // Returns a MigrationList with every enabled data type in its own
   // set.
-  MigrationList GetPreferredDataTypesList() {
+  MigrationList GetEligibleDataTypeMigrationList() {
     MigrationList migration_list;
-    const syncer::DataTypeSet preferred_data_types = GetPreferredDataTypes();
-    for (syncer::DataType type : preferred_data_types) {
+    const syncer::DataTypeSet types =
+        GetPreferredDataTypesEligibleForMigration();
+    for (syncer::DataType type : types) {
       migration_list.push_back(MakeSet(type));
     }
     return migration_list;
@@ -407,11 +413,17 @@
 // Two data types with one being nigori.
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest, PrefsNigoriIndividiaully) {
+  if (!UseGcDirective()) {
+    GTEST_SKIP() << "NIGORI migration is disallowed for MIGRATION_DONE.";
+  }
   RunSingleClientMigrationTest(MakeList(syncer::PREFERENCES, syncer::NIGORI),
                                TRIGGER_REFRESH);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest, PrefsNigoriBoth) {
+  if (!UseGcDirective()) {
+    GTEST_SKIP() << "NIGORI migration is disallowed for MIGRATION_DONE.";
+  }
   RunSingleClientMigrationTest(
       MakeList(MakeSet(syncer::PREFERENCES, syncer::NIGORI)), MODIFY_PREF);
 }
@@ -419,40 +431,49 @@
 // The whole shebang -- all data types.
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest, AllTypesIndividually) {
   ASSERT_TRUE(SetupClients());
-  RunSingleClientMigrationTest(GetPreferredDataTypesList(), MODIFY_BOOKMARK);
+  RunSingleClientMigrationTest(GetEligibleDataTypeMigrationList(),
+                               MODIFY_BOOKMARK);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest,
                        AllTypesIndividuallyTriggerRefresh) {
   ASSERT_TRUE(SetupClients());
-  RunSingleClientMigrationTest(GetPreferredDataTypesList(), TRIGGER_REFRESH);
+  RunSingleClientMigrationTest(GetEligibleDataTypeMigrationList(),
+                               TRIGGER_REFRESH);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest, AllTypesAtOnce) {
   ASSERT_TRUE(SetupClients());
-  RunSingleClientMigrationTest(MakeList(GetPreferredDataTypes()), MODIFY_PREF);
+  RunSingleClientMigrationTest(
+      MakeList(GetPreferredDataTypesEligibleForMigration()), MODIFY_PREF);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest,
                        AllTypesAtOnceTriggerRefresh) {
   ASSERT_TRUE(SetupClients());
-  RunSingleClientMigrationTest(MakeList(GetPreferredDataTypes()),
-                               TRIGGER_REFRESH);
+  RunSingleClientMigrationTest(
+      MakeList(GetPreferredDataTypesEligibleForMigration()), TRIGGER_REFRESH);
 }
 
 // All data types plus nigori.
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest,
                        AllTypesWithNigoriIndividually) {
+  if (!UseGcDirective()) {
+    GTEST_SKIP() << "NIGORI migration is disallowed for MIGRATION_DONE.";
+  }
   ASSERT_TRUE(SetupClients());
-  MigrationList migration_list = GetPreferredDataTypesList();
+  MigrationList migration_list = GetEligibleDataTypeMigrationList();
   migration_list.push_front(MakeSet(syncer::NIGORI));
   RunSingleClientMigrationTest(migration_list, MODIFY_BOOKMARK);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest, AllTypesWithNigoriAtOnce) {
+  if (!UseGcDirective()) {
+    GTEST_SKIP() << "NIGORI migration is disallowed for MIGRATION_DONE.";
+  }
   ASSERT_TRUE(SetupClients());
-  syncer::DataTypeSet all_types = GetPreferredDataTypes();
+  syncer::DataTypeSet all_types = GetPreferredDataTypesEligibleForMigration();
   all_types.Put(syncer::NIGORI);
   RunSingleClientMigrationTest(MakeList(all_types), MODIFY_PREF);
 }
@@ -819,17 +840,19 @@
 // will only tell the client about the migrations one at a time.
 IN_PROC_BROWSER_TEST_P(MigrationTwoClientTest, MigrationHellWithoutNigori) {
   ASSERT_TRUE(SetupClients());
-  MigrationList migration_list = GetPreferredDataTypesList();
+  MigrationList migration_list = GetEligibleDataTypeMigrationList();
   // Let the first nudge be a datatype that's neither prefs nor bookmarks.
   migration_list.push_front(MakeSet(syncer::THEMES));
-  ASSERT_EQ(MakeSet(syncer::NIGORI), migration_list.back());
-  migration_list.pop_back();
+  base::Erase(migration_list, MakeSet(syncer::NIGORI));
   RunTwoClientMigrationTest(migration_list, MODIFY_BOOKMARK);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationTwoClientTest, MigrationHellWithNigori) {
+  if (!UseGcDirective()) {
+    GTEST_SKIP() << "NIGORI migration is disallowed for MIGRATION_DONE.";
+  }
   ASSERT_TRUE(SetupClients());
-  MigrationList migration_list = GetPreferredDataTypesList();
+  MigrationList migration_list = GetEligibleDataTypeMigrationList();
   // Let the first nudge be a datatype that's neither prefs nor bookmarks.
   migration_list.push_front(MakeSet(syncer::THEMES));
   ASSERT_EQ(MakeSet(syncer::NIGORI), migration_list.back());
diff --git a/components/sync/engine/syncer_proto_util_unittest.cc b/components/sync/engine/syncer_proto_util_unittest.cc
index 13be8bf..d9fadd5 100644
--- a/components/sync/engine/syncer_proto_util_unittest.cc
+++ b/components/sync/engine/syncer_proto_util_unittest.cc
@@ -32,14 +32,16 @@
 }  // namespace
 
 // Builds a ClientToServerResponse with some data type ids, including
-// invalid ones.  GetTypesToMigrate() should return only the valid
-// data types.
+// invalid ones and NIGORI. GetTypesToMigrate() should return only the valid
+// data types excluding NIGORI.
 TEST(SyncerProtoUtil, GetTypesToMigrate) {
   sync_pb::ClientToServerResponse response;
   response.add_migrated_data_type_id(
       GetSpecificsFieldNumberFromDataType(BOOKMARKS));
   response.add_migrated_data_type_id(
       GetSpecificsFieldNumberFromDataType(HISTORY_DELETE_DIRECTIVES));
+  response.add_migrated_data_type_id(
+      GetSpecificsFieldNumberFromDataType(NIGORI));
   response.add_migrated_data_type_id(-1);
   EXPECT_EQ(DataTypeSet({BOOKMARKS, HISTORY_DELETE_DIRECTIVES}),
             GetTypesToMigrate(response));
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/chrome/browser/sync/test/integration/migration_test.cc b/chrome/browser/sync/test/integration/migration_test.cc
index 84f431a..e672598 100644
--- a/chrome/browser/sync/test/integration/migration_test.cc
+++ b/chrome/browser/sync/test/integration/migration_test.cc
@@ -212,7 +212,7 @@
 
   enum TriggerMethod { MODIFY_PREF, MODIFY_BOOKMARK, TRIGGER_REFRESH };
 
-  syncer::DataTypeSet GetPreferredDataTypes() {
+  syncer::DataTypeSet GetPreferredDataTypesEligibleForMigration() {
     // SyncServiceImpl must already have been created before we can call
     // GetPreferredDataTypes().
     DCHECK(GetSyncService(0));
@@ -246,15 +246,21 @@
     // Doesn't make sense to migrate commit only types.
     preferred_data_types.RemoveAll(syncer::CommitOnlyTypes());
 
+    if (!UseGcDirective()) {
+      // NIGORI migration is disallowed for MIGRATION_DONE response.
+      preferred_data_types.Remove(syncer::NIGORI);
+    }
+
     return preferred_data_types;
   }
 
   // Returns a MigrationList with every enabled data type in its own
   // set.
-  MigrationList GetPreferredDataTypesList() {
+  MigrationList GetEligibleDataTypeMigrationList() {
     MigrationList migration_list;
-    const syncer::DataTypeSet preferred_data_types = GetPreferredDataTypes();
-    for (syncer::DataType type : preferred_data_types) {
+    const syncer::DataTypeSet types =
+        GetPreferredDataTypesEligibleForMigration();
+    for (syncer::DataType type : types) {
       migration_list.push_back(MakeSet(type));
     }
     return migration_list;
@@ -407,11 +413,17 @@
 // Two data types with one being nigori.
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest, PrefsNigoriIndividiaully) {
+  if (!UseGcDirective()) {
+    GTEST_SKIP() << "NIGORI migration is disallowed for MIGRATION_DONE.";
+  }
   RunSingleClientMigrationTest(MakeList(syncer::PREFERENCES, syncer::NIGORI),
                                TRIGGER_REFRESH);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest, PrefsNigoriBoth) {
+  if (!UseGcDirective()) {
+    GTEST_SKIP() << "NIGORI migration is disallowed for MIGRATION_DONE.";
+  }
   RunSingleClientMigrationTest(
       MakeList(MakeSet(syncer::PREFERENCES, syncer::NIGORI)), MODIFY_PREF);
 }
@@ -419,40 +431,49 @@
 // The whole shebang -- all data types.
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest, AllTypesIndividually) {
   ASSERT_TRUE(SetupClients());
-  RunSingleClientMigrationTest(GetPreferredDataTypesList(), MODIFY_BOOKMARK);
+  RunSingleClientMigrationTest(GetEligibleDataTypeMigrationList(),
+                               MODIFY_BOOKMARK);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest,
                        AllTypesIndividuallyTriggerRefresh) {
   ASSERT_TRUE(SetupClients());
-  RunSingleClientMigrationTest(GetPreferredDataTypesList(), TRIGGER_REFRESH);
+  RunSingleClientMigrationTest(GetEligibleDataTypeMigrationList(),
+                               TRIGGER_REFRESH);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest, AllTypesAtOnce) {
   ASSERT_TRUE(SetupClients());
-  RunSingleClientMigrationTest(MakeList(GetPreferredDataTypes()), MODIFY_PREF);
+  RunSingleClientMigrationTest(
+      MakeList(GetPreferredDataTypesEligibleForMigration()), MODIFY_PREF);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest,
                        AllTypesAtOnceTriggerRefresh) {
   ASSERT_TRUE(SetupClients());
-  RunSingleClientMigrationTest(MakeList(GetPreferredDataTypes()),
-                               TRIGGER_REFRESH);
+  RunSingleClientMigrationTest(
+      MakeList(GetPreferredDataTypesEligibleForMigration()), TRIGGER_REFRESH);
 }
 
 // All data types plus nigori.
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest,
                        AllTypesWithNigoriIndividually) {
+  if (!UseGcDirective()) {
+    GTEST_SKIP() << "NIGORI migration is disallowed for MIGRATION_DONE.";
+  }
   ASSERT_TRUE(SetupClients());
-  MigrationList migration_list = GetPreferredDataTypesList();
+  MigrationList migration_list = GetEligibleDataTypeMigrationList();
   migration_list.push_front(MakeSet(syncer::NIGORI));
   RunSingleClientMigrationTest(migration_list, MODIFY_BOOKMARK);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationSingleClientTest, AllTypesWithNigoriAtOnce) {
+  if (!UseGcDirective()) {
+    GTEST_SKIP() << "NIGORI migration is disallowed for MIGRATION_DONE.";
+  }
   ASSERT_TRUE(SetupClients());
-  syncer::DataTypeSet all_types = GetPreferredDataTypes();
+  syncer::DataTypeSet all_types = GetPreferredDataTypesEligibleForMigration();
   all_types.Put(syncer::NIGORI);
   RunSingleClientMigrationTest(MakeList(all_types), MODIFY_PREF);
 }
@@ -819,17 +840,19 @@
 // will only tell the client about the migrations one at a time.
 IN_PROC_BROWSER_TEST_P(MigrationTwoClientTest, MigrationHellWithoutNigori) {
   ASSERT_TRUE(SetupClients());
-  MigrationList migration_list = GetPreferredDataTypesList();
+  MigrationList migration_list = GetEligibleDataTypeMigrationList();
   // Let the first nudge be a datatype that's neither prefs nor bookmarks.
   migration_list.push_front(MakeSet(syncer::THEMES));
-  ASSERT_EQ(MakeSet(syncer::NIGORI), migration_list.back());
-  migration_list.pop_back();
+  base::Erase(migration_list, MakeSet(syncer::NIGORI));
   RunTwoClientMigrationTest(migration_list, MODIFY_BOOKMARK);
 }
 
 IN_PROC_BROWSER_TEST_P(MigrationTwoClientTest, MigrationHellWithNigori) {
+  if (!UseGcDirective()) {
+    GTEST_SKIP() << "NIGORI migration is disallowed for MIGRATION_DONE.";
+  }
   ASSERT_TRUE(SetupClients());
-  MigrationList migration_list = GetPreferredDataTypesList();
+  MigrationList migration_list = GetEligibleDataTypeMigrationList();
   // Let the first nudge be a datatype that's neither prefs nor bookmarks.
   migration_list.push_front(MakeSet(syncer::THEMES));
   ASSERT_EQ(MakeSet(syncer::NIGORI), migration_list.back());
diff --git a/components/sync/engine/syncer_proto_util_unittest.cc b/components/sync/engine/syncer_proto_util_unittest.cc
index 13be8bf..d9fadd5 100644
--- a/components/sync/engine/syncer_proto_util_unittest.cc
+++ b/components/sync/engine/syncer_proto_util_unittest.cc
@@ -32,14 +32,16 @@
 }  // namespace
 
 // Builds a ClientToServerResponse with some data type ids, including
-// invalid ones.  GetTypesToMigrate() should return only the valid
-// data types.
+// invalid ones and NIGORI. GetTypesToMigrate() should return only the valid
+// data types excluding NIGORI.
 TEST(SyncerProtoUtil, GetTypesToMigrate) {
   sync_pb::ClientToServerResponse response;
   response.add_migrated_data_type_id(
       GetSpecificsFieldNumberFromDataType(BOOKMARKS));
   response.add_migrated_data_type_id(
       GetSpecificsFieldNumberFromDataType(HISTORY_DELETE_DIRECTIVES));
+  response.add_migrated_data_type_id(
+      GetSpecificsFieldNumberFromDataType(NIGORI));
   response.add_migrated_data_type_id(-1);
   EXPECT_EQ(DataTypeSet({BOOKMARKS, HISTORY_DELETE_DIRECTIVES}),
             GetTypesToMigrate(response));
Loading diff…

Original Bug Report

reported by vm...@google.com

Potential silent sync encryption downgrade via NIGORI migration handling (via MIGRATION_DONE)

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. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.

Overview: A logic flaw in Chrome Sync allows a malicious server or MITM to silently downgrade a user’s encryption from a custom passphrase to server-known keys. By triggering a migration for the NIGORI data type, an attacker can force a local state wipe that erases custom passphrase protection, allowing a subsequent transition to keystore encryption without user notification.

Affected files:

  • components/sync/service/data_type_manager_impl.cc
  • components/sync/engine/syncer_proto_util.cc
  • components/sync/nigori/nigori_sync_bridge_impl.cc
  • components/sync/service/backend_migrator.cc
  • components/sync/service/glue/sync_engine_backend.cc

Estimated timestamp from git blame: Unknown (Google3 checkout)

Summary

A logic vulnerability in Chrome Sync’s migration handling could allow an attacker who controls the sync server response (or a compromised network process) to silently strip a user’s end-to-end encryption (CUSTOM_PASSPHRASE) and downgrade it to a server-known KEYSTORE_PASSPHRASE. This effectively allows the server to decrypt sensitive user data synced after the downgrade, such as passwords and Wi-Fi credentials.

Technical Details

The vulnerability exists because the sync client’s migration logic does not adequately protect the NIGORI control type (which manages encryption keys) from being targeted for migration by the server.

Root Cause

  1. Migration Trigger: When a sync server returns error_code = MIGRATION_DONE with migrated_data_type_id containing the field number for NigoriSpecifics (47745), the client processes this as a request to migrate the NIGORI data type.
    • Reference: components/sync/engine/syncer_proto_util.cc:311 calls GetTypesToMigrate(response).
  2. State Purge: The BackendMigrator initiates a purge of the type to be migrated. For NIGORI, this causes DataTypeManagerImpl to call configurer_->ClearNigoriDataForMigration().
    • Reference: components/sync/service/data_type_manager_impl.cc:550.
  3. Local Wipe: This call reaches NigoriSyncBridgeImpl::ApplyDisableSyncChanges(), which wipes the local cryptographer and resets the passphrase_type to UNKNOWN. This effectively erases the client’s knowledge that a custom passphrase was ever in use.
    • Reference: components/sync/nigori/nigori_sync_bridge_impl.cc:930-945.
  4. Silent Re-encryption: After the purge, BackendMigrator re-enables NIGORI for reconfiguration. During the subsequent sync cycle, the client downloads the state from the server. Because the local passphrase_type is now UNKNOWN, the bridge accepts a transition to KEYSTORE_PASSPHRASE provided by the server, as transitions from UNKNOWN to any valid type are permitted.
    • Reference: components/sync/nigori/nigori_sync_bridge_impl.cc:187-193 in IsValidPassphraseTransition.

Potential Attack Scenario

An attacker controlling the sync response (compromised server or MITM) could follow these suggested steps:

  1. Serve a ClientToServerResponse with error_code = MIGRATION_DONE and migrated_data_type_id = [47745].
  2. The client will silently wipe its local encryption state and custom passphrase keys.
  3. The client re-enables sync for the NIGORI type.
  4. In the next sync update, serve NigoriSpecifics with passphrase_type = KEYSTORE_PASSPHRASE and new keys known to the server.
  5. The client accepts this transition silently. All future sensitive data (passwords, etc.) will be encrypted with the server’s keys.

Impact

This allows a compromised backend or network process to escalate its influence to the browser process and decrypt user credentials. The attack is entirely silent; no engine reset or UI prompts are visible to the user.

Suggested Fix

The sync client should explicitly prevent the NIGORI data type from being targeted for migration by the server. In components/sync/engine/syncer_proto_util.cc, GetTypesToMigrate should filter out control types like NIGORI. Additionally, NigoriSyncBridgeImpl should be hardened to prevent silent transitions away from CUSTOM_PASSPHRASE even if the local state has been cleared for migration, perhaps by persisting a ‘was custom passphrase’ bit across migration purges.

Evaluated with Chrome root at commit: 1a8d40fc44df2088d5945c0bf53584038aa1614a


Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:

  • If you are familiar with the severity guidelines, you may adjust the severity.
  • If this is a false positive, and there’s no work to be done, please close as WAI.
  • If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.

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