CVE-2026-79013
Overview
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
chrome/browser/sync/test/integration/migration_test.cccomponents/sync/engine/syncer_proto_util.cccomponents/sync/engine/syncer_proto_util_unittest.cc
Patch
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/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.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/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.