CVE-2026-79013
Overview
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.ccthat turns the server-suppliedmigrated_data_type_idfield numbers into theDataTypeSetthe 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}.
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
- Control sync server responses
The attacker must be able to shape
ClientToServerResponsemessages, 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). - Send `MIGRATION_DONE` naming NIGORI
The response has the
MIGRATION_DONEerror, withmigrated_data_type_idincluding the specifics field number forNIGORI. - Client schedules NIGORI migration
Before the fix,
GetTypesToMigrate()returned a set containingNIGORI, and the client purged its local encryption state and downloaded it again. - 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
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
| Function | Change | Notes |
|---|---|---|
forchrome/browser/sync/test/integration/migration_test.cc |
modified | |
IN_PROC_BROWSER_TEST_Pchrome/browser/sync/test/integration/migration_test.cc |
modified | |
TESTcomponents/sync/engine/syncer_proto_util_unittest.cc |
modified |
Files Changed
components/sync/engine/syncer_proto_util_unittest.cccomponents/sync/engine/syncer_proto_util.ccchrome/browser/sync/test/integration/migration_test.cc
Audit Directions
- Server-driven reset of security-critical typesReview 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 replaceNIGORIor passphrase state without dedicated handling. - Allow-lists for type sets parsed from protobufsWhere 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 transitionsAudit 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.
Patch
From dcebabc243ac3fc05519fe8d52fea65ac2490c3b Mon Sep 17 00:00:00 2001 From: Mikel Astiz <[email protected]> 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 <[email protected]> Commit-Queue: Mikel Astiz <[email protected]> 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));
Regression Test / PoC
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));
Original Bug Report
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.cccomponents/sync/engine/syncer_proto_util.cccomponents/sync/nigori/nigori_sync_bridge_impl.cccomponents/sync/service/backend_migrator.cccomponents/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
- Migration Trigger: When a sync server returns
error_code = MIGRATION_DONEwithmigrated_data_type_idcontaining the field number forNigoriSpecifics(47745), the client processes this as a request to migrate theNIGORIdata type.- Reference:
components/sync/engine/syncer_proto_util.cc:311callsGetTypesToMigrate(response).
- Reference:
- State Purge: The
BackendMigratorinitiates a purge of the type to be migrated. ForNIGORI, this causesDataTypeManagerImplto callconfigurer_->ClearNigoriDataForMigration().- Reference:
components/sync/service/data_type_manager_impl.cc:550.
- Reference:
- Local Wipe: This call reaches
NigoriSyncBridgeImpl::ApplyDisableSyncChanges(), which wipes the local cryptographer and resets thepassphrase_typetoUNKNOWN. 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.
- Reference:
- Silent Re-encryption: After the purge,
BackendMigratorre-enablesNIGORIfor reconfiguration. During the subsequent sync cycle, the client downloads the state from the server. Because the localpassphrase_typeis nowUNKNOWN, the bridge accepts a transition toKEYSTORE_PASSPHRASEprovided by the server, as transitions fromUNKNOWNto any valid type are permitted.- Reference:
components/sync/nigori/nigori_sync_bridge_impl.cc:187-193inIsValidPassphraseTransition.
- Reference:
Potential Attack Scenario
An attacker controlling the sync response (compromised server or MITM) could follow these suggested steps:
- Serve a
ClientToServerResponsewitherror_code = MIGRATION_DONEandmigrated_data_type_id = [47745]. - The client will silently wipe its local encryption state and custom passphrase keys.
- The client re-enables sync for the
NIGORItype. - In the next sync update, serve
NigoriSpecificswithpassphrase_type = KEYSTORE_PASSPHRASEand new keys known to the server. - 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.