PatchGap
Chrome · Sync › TrustedVault

CVE-2026-79076

Input Validation in Sync (TrustedVault)
Medium watch CVSS 6.5 chrome Input Validation 📄 Reporter bug report 🔧 Commit mapped

Overview

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

Background

Trusted vault
the client-side component (components/trusted_vault) that downloads and validates the end-to-end-encryption (E2EE) keys protecting Chrome Sync data from Google’s security-domain server, under a threat model in which the server itself is not trusted with key material.
Key chain and rotation proofs
each trusted vault key carries a monotonically increasing version, and every new key must carry a rotation proof signed with the previous key, so a client can verify a chain from its last known key (last_trusted_vault_key_and_version_) to the newest key.
`DownloadKeysResponseHandler`
the class that parses the server’s SecurityDomainMember response, extracts the wrapped keys (extracted_keys), validates them, and returns a ProcessedResponse with a TrustedVaultDownloadKeysStatus such as kSuccess, kNoNewKeys, kMembershipCorrupted or kKeyProofsVerificationFailed.
`kE2eeRotationProofBypassFix`
a base::Feature (already FEATURE_ENABLED_BY_DEFAULT before this commit) that gated the stricter validation; this commit deletes the flag and the legacy code path it guarded.

Root Cause Analysis

In the legacy branch (flag disabled), the response handler called the key-chain validator unconditionally and then classified the result as kNoNewKeys whenever extracted_keys->back().version <= last_trusted_vault_key_and_version_.version; the removed comment says the <= was deliberately tolerant “to make the legacy implementation more robust”. Because rotation proofs only exist for keys newer than the last trusted one, a response whose newest key had the same or a lower version passed without any cryptographic check, and the server-supplied key bytes were returned in downloaded_keys. The removed test comments state the consequence directly: the legacy path “accepts a different key at the same version without verification, incorrectly returning kNoNewKeys with the server-supplied wrong key”, and “tolerates version regression … with the regressed lower-version key”. The fixed logic, now unconditional,

  1. rejects any response whose newest version is below the last trusted version as kMembershipCorrupted,
  2. rejects a response that contains the last trusted version but with different key bytes as kMembershipCorrupted,
  3. runs rotation-proof verification only when there are genuinely newer keys and returns kKeyProofsVerificationFailed on failure, and
  4. reports kNoNewKeys only on strict version equality. The validator also gains an unconditional CHECK that the chain’s newest version is strictly greater than the last known version, encoding the new calling invariant. This commit is a flag cleanup: the protective logic had already shipped enabled by default, and here the vulnerable legacy path is removed entirely so it can no longer be re-enabled through the feature system.
Key insight
The legacy handler treated “no newer version” as “nothing to verify” and passed through whatever key bytes the server sent at an equal or lower version, so the server could replace the client’s trusted key without a rotation proof. The fix binds equal-version keys to the known key bytes, rejects version regression, and keeps proof verification for strictly newer keys only.

Attack Path

  1. Attacker controls the key response An attacker able to control or tamper with the security-domain server’s SecurityDomainMember response for the user’s sync domain (a malicious or compromised server side, which the E2EE design treats as untrusted) supplies a crafted membership. Reaching this position is a strong precondition and is not shown by the patch.
  2. Craft an unverifiable key The response contains a key whose version equals the client’s last trusted version but with different bytes, or a key with a lower version, and no rotation proof, matching the ShouldHandleSameVersionWithDifferentKey and ShouldHandleVersionRegression tests.
  3. Legacy handler accepts it With kE2eeRotationProofBypassFix disabled, the <= comparison classified the response as kNoNewKeys and returned the attacker-supplied key in downloaded_keys without any rotation-proof check.
  4. Client adopts attacker key material The client would then treat the substituted key as a trusted vault key for Sync (inferred), which could let the server side read data the client later encrypts with it or anchor later forged rotations to it (inferred).

Impact Assessment

The bug weakens Chrome Sync’s end-to-end encryption guarantee: a party controlling the trusted vault server’s responses could get the client to accept substituted or rolled-back key material without cryptographic proof. It runs in the browser process’s sync/trusted-vault logic, needs no renderer compromise, but does need control of the server-side key response, and before this commit it was only reachable when the default-on kE2eeRotationProofBypassFix feature was disabled (for example by a field trial or command-line override). Decryption of user sync data or persistent key compromise is (inferred); the advisory lists only “Improper input validation in Sync” and gives no exploitation status.

Changed Functions

FunctionChangeNotes
if
components/trusted_vault/download_keys_response_handler.cc
 modified
DownloadKeysResponseHandlerTest
components/trusted_vault/download_keys_response_handler_unittest.cc
 modified
TEST_P
components/trusted_vault/download_keys_response_handler_unittest.cc
 modified
TEST_F
components/trusted_vault/download_keys_response_handler_unittest.cc
 modified
for
components/trusted_vault/download_keys_response_handler_unittest.cc
 modified

Files Changed

  • components/trusted_vault/download_keys_response_handler.cc
  • components/trusted_vault/features.h
  • components/trusted_vault/download_keys_response_handler_unittest.cc
  • components/trusted_vault/features.cc

Audit Directions

  • "Nothing new, so nothing to verify" shortcuts
    In any versioned key or certificate chain, check that equal-version or lower-version server data is compared against locally pinned material rather than skipped, and that returned data comes from the local trusted copy, not the response.
  • Tolerant comparisons kept for robustness
    Look for <= or >= comparisons on versions, epochs or counters that a comment justifies as being tolerant, since they often accept rollback; prefer explicit rejection plus a distinct error status.
  • Security fixes left behind kill switches
    Audit base::Feature flags wrapping security fixes (other trusted vault, sync encryption or key-management flags) for legacy branches that remain reachable through field trials or command-line overrides, and confirm they are cleaned up once rolled out.

Patch

view commit
From a63b8a215549dc9dc196c2e33fb2c047d98ec21c Mon Sep 17 00:00:00 2001
From: Thomas Thrainer <thomasth@google.com>
Date: Wed, 22 Jul 2026 03:35:09 -0700
Subject: [PATCH] [TrustedVault] Clean up kE2eeRotationProofBypassFix feature flag

Remove the fully rolled out `kE2eeRotationProofBypassFix` flag and
inline changes to `DownloadKeysResponseHandler`.

Fixed: 496395158
Test: components_unittests --gtest_filter=DownloadKeysResponseHandlerTest.*
Change-Id: Ic6763ba271c297a97eba0627563b6ce46a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8061799
Commit-Queue: Thomas Thrainer <thomasth@google.com>
Reviewed-by: Adem Derinel <derinel@google.com>
Cr-Commit-Position: refs/heads/main@{#1666153}
---
diff --git a/components/trusted_vault/download_keys_response_handler.cc b/components/trusted_vault/download_keys_response_handler.cc
index a16191b4..936023e0 100644
--- a/components/trusted_vault/download_keys_response_handler.cc
+++ b/components/trusted_vault/download_keys_response_handler.cc
@@ -10,7 +10,6 @@
 #include <utility>
 
 #include "base/check.h"
-#include "components/trusted_vault/features.h"
 #include "components/trusted_vault/proto/vault.pb.h"
 #include "components/trusted_vault/proto_string_bytes_conversion.h"
 #include "components/trusted_vault/securebox.h"
@@ -92,10 +91,8 @@
   CHECK(!key_chain.empty());
   CHECK(std::ranges::is_sorted(key_chain, std::less<>(),
                                &ExtractedSharedKey::version));
-  if (base::FeatureList::IsEnabled(kE2eeRotationProofBypassFix)) {
-    CHECK(key_chain.back().version >
-          last_known_trusted_vault_key_and_version.version);
-  }
+  CHECK(key_chain.back().version >
+        last_known_trusted_vault_key_and_version.version);
 
   int last_valid_key_version = last_known_trusted_vault_key_and_version.version;
   std::vector<uint8_t> last_valid_key =
@@ -222,46 +219,34 @@
     return ProcessedResponse(
         /*status=*/TrustedVaultDownloadKeysStatus::kMembershipEmpty);
   }
-  if (base::FeatureList::IsEnabled(kE2eeRotationProofBypassFix)) {
-    if (extracted_keys->back().version <
-        last_trusted_vault_key_and_version_.version) {
-      // |current_member| appears corrupt, as its keys have a lower version than
-      // the last currently trusted one.
-      return ProcessedResponse(
-          /*status=*/TrustedVaultDownloadKeysStatus::kMembershipCorrupted);
-    }
+  if (extracted_keys->back().version <
+      last_trusted_vault_key_and_version_.version) {
+    // |current_member| appears corrupt, as its keys have a lower version than
+    // the last currently trusted one.
+    return ProcessedResponse(
+        /*status=*/TrustedVaultDownloadKeysStatus::kMembershipCorrupted);
+  }
 
-    if (auto it = std::ranges::find(*extracted_keys,
-                                    last_trusted_vault_key_and_version_.version,
-                                    &ExtractedSharedKey::version);
-        it != extracted_keys->end() &&
-        it->trusted_vault_key != last_trusted_vault_key_and_version_.key) {
-      // |current_member| appears corrupt, as its key with the version of the
-      // last trusted vault key doesn't match the key bytes of the last
-      // trusted vault key.
-      return ProcessedResponse(
-          /*status=*/TrustedVaultDownloadKeysStatus::kMembershipCorrupted);
-    }
+  if (auto it = std::ranges::find(*extracted_keys,
+                                  last_trusted_vault_key_and_version_.version,
+                                  &ExtractedSharedKey::version);
+      it != extracted_keys->end() &&
+      it->trusted_vault_key != last_trusted_vault_key_and_version_.key) {
+    // |current_member| appears corrupt, as its key with the version of the
+    // last trusted vault key doesn't match the key bytes of the last
+    // trusted vault key.
+    return ProcessedResponse(
+        /*status=*/TrustedVaultDownloadKeysStatus::kMembershipCorrupted);
+  }
 
-    if (extracted_keys->back().version >
-            last_trusted_vault_key_and_version_.version &&
-        !IsValidKeyChain(*extracted_keys,
-                         last_trusted_vault_key_and_version_)) {
-      // New keys provided by |current_member| failed rotation proof
-      // verification.
-      return ProcessedResponse(
-          /*status=*/TrustedVaultDownloadKeysStatus::
-              kKeyProofsVerificationFailed);
-    }
-  } else {
-    if (!IsValidKeyChain(*extracted_keys,
-                         last_trusted_vault_key_and_version_)) {
-      // Data corresponding to |current_member| is corrupted or
-      // |last_trusted_vault_key_and_version_| is too old.
-      return ProcessedResponse(
-          /*status=*/TrustedVaultDownloadKeysStatus::
-              kKeyProofsVerificationFailed);
-    }
+  if (extracted_keys->back().version >
+          last_trusted_vault_key_and_version_.version &&
+      !IsValidKeyChain(*extracted_keys, last_trusted_vault_key_and_version_)) {
+    // New keys provided by |current_member| failed rotation proof
+    // verification.
+    return ProcessedResponse(
+        /*status=*/TrustedVaultDownloadKeysStatus::
+            kKeyProofsVerificationFailed);
   }
 
   std::vector<std::vector<uint8_t>> trusted_vault_keys;
@@ -271,19 +256,8 @@
 
   TrustedVaultDownloadKeysStatus status =
       TrustedVaultDownloadKeysStatus::kSuccess;
-  const bool is_no_new_keys =
-      base::FeatureList::IsEnabled(kE2eeRotationProofBypassFix)
-          ? (extracted_keys->back().version ==
-             last_trusted_vault_key_and_version_.version)
-          : (extracted_keys->back().version <=
-             last_trusted_vault_key_and_version_.version);
-  if (is_no_new_keys) {
-    // In theory, the check when the feature flag is disabled could be ==
-    // instead of <=, since server version should not decrease, but it was
-    // tolerated to make the legacy implementation more robust. When the
-    // `kE2eeRotationProofBypassFix` flag is enabled, strictly lower version
-    // cases are already rejected in the conditions above, so checking for
-    // strict equality here is sufficient and more consistent.
+  if (extracted_keys->back().version ==
+      last_trusted_vault_key_and_version_.version) {
     status = TrustedVaultDownloadKeysStatus::kNoNewKeys;
   }
 
diff --git a/components/trusted_vault/features.cc b/components/trusted_vault/features.cc
index 25255bf..e667047 100644
--- a/components/trusted_vault/features.cc
+++ b/components/trusted_vault/features.cc
@@ -9,9 +9,5 @@
 
 namespace trusted_vault {
 
-
 BASE_FEATURE(kEnableTrustedVaultSHA256, base::FEATURE_ENABLED_BY_DEFAULT);
-
-BASE_FEATURE(kE2eeRotationProofBypassFix, base::FEATURE_ENABLED_BY_DEFAULT);
-
 }  // namespace trusted_vault
diff --git a/components/trusted_vault/features.h b/components/trusted_vault/features.h
index 9c15e253..f228efd9 100644
--- a/components/trusted_vault/features.h
+++ b/components/trusted_vault/features.h
@@ -11,13 +11,9 @@
 
 namespace trusted_vault {
 
-
 // TODO(crug.com/425990763): Complete MD5 -> SHA256 migration.
 BASE_DECLARE_FEATURE(kEnableTrustedVaultSHA256);
 
-// Enables the security fix for E2EE rotation proof bypass in Trusted Vault.
-BASE_DECLARE_FEATURE(kE2eeRotationProofBypassFix);
-
 }  // namespace trusted_vault
 
 #endif  // COMPONENTS_TRUSTED_VAULT_FEATURES_H_
diff --git a/components/trusted_vault/download_keys_response_handler_unittest.cc b/components/trusted_vault/download_keys_response_handler_unittest.cc
index e542164..2470cc9 100644
--- a/components/trusted_vault/download_keys_response_handler_unittest.cc
+++ b/components/trusted_vault/download_keys_response_handler_unittest.cc
@@ -7,8 +7,6 @@
 #include <vector>
 
 #include "base/strings/string_number_conversions.h"
-#include "base/test/scoped_feature_list.h"
-#include "components/trusted_vault/features.h"
 #include "components/trusted_vault/proto/vault.pb.h"
 #include "components/trusted_vault/proto_string_bytes_conversion.h"
 #include "components/trusted_vault/securebox.h"
@@ -82,19 +80,13 @@
   return member.SerializeAsString();
 }
 
-class DownloadKeysResponseHandlerTest : public testing::TestWithParam<bool> {
+class DownloadKeysResponseHandlerTest : public testing::Test {
  public:
   DownloadKeysResponseHandlerTest()
       : handler_(SecurityDomainId::kChromeSync,
                  TrustedVaultKeyAndVersion(kKnownTrustedVaultKey,
                                            kKnownTrustedVaultKeyVersion),
-                 MakeTestKeyPair()) {
-    if (GetParam()) {
-      scoped_feature_list_.InitAndEnableFeature(kE2eeRotationProofBypassFix);
-    } else {
-      scoped_feature_list_.InitAndDisableFeature(kE2eeRotationProofBypassFix);
-    }
-  }
+                 MakeTestKeyPair()) {}
 
   ~DownloadKeysResponseHandlerTest() override = default;
 
@@ -107,15 +99,12 @@
   const std::vector<uint8_t> kTrustedVaultKey3 = {1, 2, 3, 7};
 
  private:
-  base::test::ScopedFeatureList scoped_feature_list_;
   const DownloadKeysResponseHandler handler_;
 };
 
-INSTANTIATE_TEST_SUITE_P(All, DownloadKeysResponseHandlerTest, testing::Bool());
-
 // All HttpStatuses except kSuccess should end up in kOtherError, kNetworkError
 // or kMemberNotFound reporting.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleHttpErrors) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleHttpErrors) {
   EXPECT_THAT(
       handler()
           .ProcessResponse(
@@ -155,7 +144,7 @@
 
 // Simplest legitimate case of key rotation, server side state corresponds to
 // kKnownTrustedVaultKey -> kTrustedVaultKey1 key chain.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleSingleKeyRotation) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleSingleKeyRotation) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
           /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
@@ -176,7 +165,7 @@
 
 // Multiple key rotations may happen while client is offline, server-side key
 // chain is kKnownTrustedVaultKey -> kTrustedVaultKey1 -> kTrustedVaultKey2.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleMultipleKeyRotations) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleMultipleKeyRotations) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
           /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
@@ -203,7 +192,7 @@
 // Full key chain is: kKnownTrustedVaultKey -> kTrustedVaultKey1 ->
 // kTrustedVaultKey2.
 // Server-side key chain is: kTrustedVaultKey1 -> kTrustedVaultKey2.
-TEST_P(DownloadKeysResponseHandlerTest,
+TEST_F(DownloadKeysResponseHandlerTest,
        ShouldHandleAbsenseOfKnownKeyWhenKeyChainIsRecoverable) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
@@ -232,7 +221,7 @@
 // Possible full key chain is: kKnownTrustedVaultKey -> kTrustedVaultKey1 ->
 // kTrustedVaultKey2 -> kTrustedVaultKey3.
 // Server side key chain is: kTrustedVaultKey2 -> kTrustedVaultKey3.
-TEST_P(DownloadKeysResponseHandlerTest,
+TEST_F(DownloadKeysResponseHandlerTest,
        ShouldHandleAbsenseOfKnownKeyWhenKeyChainIsNotRecoverable) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
@@ -255,7 +244,7 @@
 // The test populates undecryptable/corrupted |wrapped_key| field, handler
 // should return kMembershipCorrupted to allow client to restore the member by
 // re-registration.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleUndecryptableKey) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleUndecryptableKey) {
   trusted_vault_pb::SecurityDomainMember member;
   AddSecurityDomainMembership(
       GetSecurityDomainPath(SecurityDomainId::kChromeSync),
@@ -280,7 +269,7 @@
 // The test populates invalid |rotation_proof| field for the single key
 // rotation. kTrustedVaultKey1 is expected to be signed with
 // kKnownTrustedVaultKey, but instead it's signed with kTrustedVaultKey2.
-TEST_P(DownloadKeysResponseHandlerTest,
+TEST_F(DownloadKeysResponseHandlerTest,
        ShouldHandleInvalidKeyProofOnSingleKeyRotation) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
@@ -301,7 +290,7 @@
 // multiple key rotations have happened.
 // kTrustedVaultKey1 is expected to be signed with kKnownTrustedVaultKey, but
 // instead it's signed with kTrustedVaultKey2.
-TEST_P(DownloadKeysResponseHandlerTest,
+TEST_F(DownloadKeysResponseHandlerTest,
        ShouldHandleInvalidKeyProofOnMultipleKeyRotations) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
@@ -321,7 +310,7 @@
 }
 
 // In this scenario client already has most recent trusted vault key.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfNewKeys) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfNewKeys) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
           /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
@@ -339,7 +328,7 @@
 
 // Tests handling the situation, when response isn't a valid serialized
 // SecurityDomainMemberProto proto.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleCorruptedResponseProto) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleCorruptedResponseProto) {
   EXPECT_THAT(handler()
                   .ProcessResponse(
                       /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
@@ -350,7 +339,7 @@
 
 // Client expects that the sync security domain membership exists, but the
 // response indicates it doesn't by having no memberships.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfMemberships) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfMemberships) {
   EXPECT_THAT(handler()
                   .ProcessResponse(
                       /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
@@ -361,7 +350,7 @@
 }
 
 // Same as above, but there is a different security domain membership.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfSyncMembership) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfSyncMembership) {
   trusted_vault_pb::SecurityDomainMember member;
   AddSecurityDomainMembership(
       "other_domain", MakeTestKeyPair()->public_key(),
@@ -377,7 +366,7 @@
               Eq(TrustedVaultDownloadKeysStatus::kMembershipNotFound));
 }
 
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleEmptyMembership) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleEmptyMembership) {
   trusted_vault_pb::SecurityDomainMember member;
   AddSecurityDomainMembership(
       GetSecurityDomainPath(SecurityDomainId::kChromeSync),
@@ -394,7 +383,7 @@
               Eq(TrustedVaultDownloadKeysStatus::kMembershipEmpty));
 }
 
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleAllSecurityDomains) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleAllSecurityDomains) {
   for (const SecurityDomainId security_domain : kAllSecurityDomainIdValues) {
     trusted_vault_pb::SecurityDomainMember member;
     AddSecurityDomainMembership(
@@ -423,7 +412,7 @@
 }
 
 // Tests handling presence of other security domain memberships.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleMultipleSecurityDomains) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleMultipleSecurityDomains) {
   trusted_vault_pb::SecurityDomainMember member;
   AddSecurityDomainMembership(
       "other_domain", MakeTestKeyPair()->public_key(),
@@ -453,7 +442,7 @@
               Eq(kKnownTrustedVaultKeyVersion + 1));
 }
 
-TEST_P(DownloadKeysResponseHandlerTest,
+TEST_F(DownloadKeysResponseHandlerTest,
        ShouldHandleSameVersionWithDifferentKey) {
   // Server sends a different key at same version, with no rotation proof.
   const std::vector<uint8_t> kDifferentKey = {9, 9, 9, 9};
@@ -466,22 +455,12 @@
               /*trusted_vault_keys_versions=*/{kKnownTrustedVaultKeyVersion},
               /*signing_keys=*/{{}}));
 
-  if (GetParam()) {
-    EXPECT_THAT(processed_response.status,
-                Eq(TrustedVaultDownloadKeysStatus::kMembershipCorrupted));
-    EXPECT_THAT(processed_response.downloaded_keys, IsEmpty());
-  } else {
-    // NOTE: The legacy implementation (when the feature flag is disabled) has
-    // a bug/vulnerability where it accepts a different key at the same version
-    // without verification, incorrectly returning `kNoNewKeys` with the
-    // server-supplied wrong key.
-    EXPECT_THAT(processed_response.status,
-                Eq(TrustedVaultDownloadKeysStatus::kNoNewKeys));
-    EXPECT_THAT(processed_response.downloaded_keys, ElementsAre(kDifferentKey));
-  }
+  EXPECT_THAT(processed_response.status,
+              Eq(TrustedVaultDownloadKeysStatus::kMembershipCorrupted));
+  EXPECT_THAT(processed_response.downloaded_keys, IsEmpty());
 }
 
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleVersionRegression) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleVersionRegression) {
   // Server sends key with a lower version, no rotation proof.
   const std::vector<uint8_t> kLowerVersionKey = {9, 9, 9, 9};
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
@@ -494,22 +473,12 @@
               {kKnownTrustedVaultKeyVersion - 1},
               /*signing_keys=*/{{}}));
 
-  if (GetParam()) {
-    EXPECT_THAT(processed_response.status,
-                Eq(TrustedVaultDownloadKeysStatus::kMembershipCorrupted));
-    EXPECT_THAT(processed_response.downloaded_keys, IsEmpty());
-  } else {
-    // NOTE: The legacy implementation (when the feature flag is disabled) has
-    // a bug/vulnerability where it tolerates version regression, incorrectly
-    // returning `kNoNewKeys` with the regressed lower-version key.
-    EXPECT_THAT(processed_response.status,
-                Eq(TrustedVaultDownloadKeysStatus::kNoNewKeys));
-    EXPECT_THAT(processed_response.downloaded_keys,
-                ElementsAre(kLowerVersionKey));
-  }
+  EXPECT_THAT(processed_response.status,
... (file diff truncated)
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/components/trusted_vault/download_keys_response_handler_unittest.cc b/components/trusted_vault/download_keys_response_handler_unittest.cc
index e542164..2470cc9 100644
--- a/components/trusted_vault/download_keys_response_handler_unittest.cc
+++ b/components/trusted_vault/download_keys_response_handler_unittest.cc
@@ -7,8 +7,6 @@
 #include <vector>
 
 #include "base/strings/string_number_conversions.h"
-#include "base/test/scoped_feature_list.h"
-#include "components/trusted_vault/features.h"
 #include "components/trusted_vault/proto/vault.pb.h"
 #include "components/trusted_vault/proto_string_bytes_conversion.h"
 #include "components/trusted_vault/securebox.h"
@@ -82,19 +80,13 @@
   return member.SerializeAsString();
 }
 
-class DownloadKeysResponseHandlerTest : public testing::TestWithParam<bool> {
+class DownloadKeysResponseHandlerTest : public testing::Test {
  public:
   DownloadKeysResponseHandlerTest()
       : handler_(SecurityDomainId::kChromeSync,
                  TrustedVaultKeyAndVersion(kKnownTrustedVaultKey,
                                            kKnownTrustedVaultKeyVersion),
-                 MakeTestKeyPair()) {
-    if (GetParam()) {
-      scoped_feature_list_.InitAndEnableFeature(kE2eeRotationProofBypassFix);
-    } else {
-      scoped_feature_list_.InitAndDisableFeature(kE2eeRotationProofBypassFix);
-    }
-  }
+                 MakeTestKeyPair()) {}
 
   ~DownloadKeysResponseHandlerTest() override = default;
 
@@ -107,15 +99,12 @@
   const std::vector<uint8_t> kTrustedVaultKey3 = {1, 2, 3, 7};
 
  private:
-  base::test::ScopedFeatureList scoped_feature_list_;
   const DownloadKeysResponseHandler handler_;
 };
 
-INSTANTIATE_TEST_SUITE_P(All, DownloadKeysResponseHandlerTest, testing::Bool());
-
 // All HttpStatuses except kSuccess should end up in kOtherError, kNetworkError
 // or kMemberNotFound reporting.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleHttpErrors) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleHttpErrors) {
   EXPECT_THAT(
       handler()
           .ProcessResponse(
@@ -155,7 +144,7 @@
 
 // Simplest legitimate case of key rotation, server side state corresponds to
 // kKnownTrustedVaultKey -> kTrustedVaultKey1 key chain.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleSingleKeyRotation) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleSingleKeyRotation) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
           /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
@@ -176,7 +165,7 @@
 
 // Multiple key rotations may happen while client is offline, server-side key
 // chain is kKnownTrustedVaultKey -> kTrustedVaultKey1 -> kTrustedVaultKey2.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleMultipleKeyRotations) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleMultipleKeyRotations) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
           /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
@@ -203,7 +192,7 @@
 // Full key chain is: kKnownTrustedVaultKey -> kTrustedVaultKey1 ->
 // kTrustedVaultKey2.
 // Server-side key chain is: kTrustedVaultKey1 -> kTrustedVaultKey2.
-TEST_P(DownloadKeysResponseHandlerTest,
+TEST_F(DownloadKeysResponseHandlerTest,
        ShouldHandleAbsenseOfKnownKeyWhenKeyChainIsRecoverable) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
@@ -232,7 +221,7 @@
 // Possible full key chain is: kKnownTrustedVaultKey -> kTrustedVaultKey1 ->
 // kTrustedVaultKey2 -> kTrustedVaultKey3.
 // Server side key chain is: kTrustedVaultKey2 -> kTrustedVaultKey3.
-TEST_P(DownloadKeysResponseHandlerTest,
+TEST_F(DownloadKeysResponseHandlerTest,
        ShouldHandleAbsenseOfKnownKeyWhenKeyChainIsNotRecoverable) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
@@ -255,7 +244,7 @@
 // The test populates undecryptable/corrupted |wrapped_key| field, handler
 // should return kMembershipCorrupted to allow client to restore the member by
 // re-registration.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleUndecryptableKey) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleUndecryptableKey) {
   trusted_vault_pb::SecurityDomainMember member;
   AddSecurityDomainMembership(
       GetSecurityDomainPath(SecurityDomainId::kChromeSync),
@@ -280,7 +269,7 @@
 // The test populates invalid |rotation_proof| field for the single key
 // rotation. kTrustedVaultKey1 is expected to be signed with
 // kKnownTrustedVaultKey, but instead it's signed with kTrustedVaultKey2.
-TEST_P(DownloadKeysResponseHandlerTest,
+TEST_F(DownloadKeysResponseHandlerTest,
        ShouldHandleInvalidKeyProofOnSingleKeyRotation) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
@@ -301,7 +290,7 @@
 // multiple key rotations have happened.
 // kTrustedVaultKey1 is expected to be signed with kKnownTrustedVaultKey, but
 // instead it's signed with kTrustedVaultKey2.
-TEST_P(DownloadKeysResponseHandlerTest,
+TEST_F(DownloadKeysResponseHandlerTest,
        ShouldHandleInvalidKeyProofOnMultipleKeyRotations) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
@@ -321,7 +310,7 @@
 }
 
 // In this scenario client already has most recent trusted vault key.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfNewKeys) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfNewKeys) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
           /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
@@ -339,7 +328,7 @@
 
 // Tests handling the situation, when response isn't a valid serialized
 // SecurityDomainMemberProto proto.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleCorruptedResponseProto) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleCorruptedResponseProto) {
   EXPECT_THAT(handler()
                   .ProcessResponse(
                       /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
@@ -350,7 +339,7 @@
 
 // Client expects that the sync security domain membership exists, but the
 // response indicates it doesn't by having no memberships.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfMemberships) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfMemberships) {
   EXPECT_THAT(handler()
                   .ProcessResponse(
                       /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
@@ -361,7 +350,7 @@
 }
 
 // Same as above, but there is a different security domain membership.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfSyncMembership) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleAbsenseOfSyncMembership) {
   trusted_vault_pb::SecurityDomainMember member;
   AddSecurityDomainMembership(
       "other_domain", MakeTestKeyPair()->public_key(),
@@ -377,7 +366,7 @@
               Eq(TrustedVaultDownloadKeysStatus::kMembershipNotFound));
 }
 
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleEmptyMembership) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleEmptyMembership) {
   trusted_vault_pb::SecurityDomainMember member;
   AddSecurityDomainMembership(
       GetSecurityDomainPath(SecurityDomainId::kChromeSync),
@@ -394,7 +383,7 @@
               Eq(TrustedVaultDownloadKeysStatus::kMembershipEmpty));
 }
 
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleAllSecurityDomains) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleAllSecurityDomains) {
   for (const SecurityDomainId security_domain : kAllSecurityDomainIdValues) {
     trusted_vault_pb::SecurityDomainMember member;
     AddSecurityDomainMembership(
@@ -423,7 +412,7 @@
 }
 
 // Tests handling presence of other security domain memberships.
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleMultipleSecurityDomains) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleMultipleSecurityDomains) {
   trusted_vault_pb::SecurityDomainMember member;
   AddSecurityDomainMembership(
       "other_domain", MakeTestKeyPair()->public_key(),
@@ -453,7 +442,7 @@
               Eq(kKnownTrustedVaultKeyVersion + 1));
 }
 
-TEST_P(DownloadKeysResponseHandlerTest,
+TEST_F(DownloadKeysResponseHandlerTest,
        ShouldHandleSameVersionWithDifferentKey) {
   // Server sends a different key at same version, with no rotation proof.
   const std::vector<uint8_t> kDifferentKey = {9, 9, 9, 9};
@@ -466,22 +455,12 @@
               /*trusted_vault_keys_versions=*/{kKnownTrustedVaultKeyVersion},
               /*signing_keys=*/{{}}));
 
-  if (GetParam()) {
-    EXPECT_THAT(processed_response.status,
-                Eq(TrustedVaultDownloadKeysStatus::kMembershipCorrupted));
-    EXPECT_THAT(processed_response.downloaded_keys, IsEmpty());
-  } else {
-    // NOTE: The legacy implementation (when the feature flag is disabled) has
-    // a bug/vulnerability where it accepts a different key at the same version
-    // without verification, incorrectly returning `kNoNewKeys` with the
-    // server-supplied wrong key.
-    EXPECT_THAT(processed_response.status,
-                Eq(TrustedVaultDownloadKeysStatus::kNoNewKeys));
-    EXPECT_THAT(processed_response.downloaded_keys, ElementsAre(kDifferentKey));
-  }
+  EXPECT_THAT(processed_response.status,
+              Eq(TrustedVaultDownloadKeysStatus::kMembershipCorrupted));
+  EXPECT_THAT(processed_response.downloaded_keys, IsEmpty());
 }
 
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleVersionRegression) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleVersionRegression) {
   // Server sends key with a lower version, no rotation proof.
   const std::vector<uint8_t> kLowerVersionKey = {9, 9, 9, 9};
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
@@ -494,22 +473,12 @@
               {kKnownTrustedVaultKeyVersion - 1},
               /*signing_keys=*/{{}}));
 
-  if (GetParam()) {
-    EXPECT_THAT(processed_response.status,
-                Eq(TrustedVaultDownloadKeysStatus::kMembershipCorrupted));
-    EXPECT_THAT(processed_response.downloaded_keys, IsEmpty());
-  } else {
-    // NOTE: The legacy implementation (when the feature flag is disabled) has
-    // a bug/vulnerability where it tolerates version regression, incorrectly
-    // returning `kNoNewKeys` with the regressed lower-version key.
-    EXPECT_THAT(processed_response.status,
-                Eq(TrustedVaultDownloadKeysStatus::kNoNewKeys));
-    EXPECT_THAT(processed_response.downloaded_keys,
-                ElementsAre(kLowerVersionKey));
-  }
+  EXPECT_THAT(processed_response.status,
+              Eq(TrustedVaultDownloadKeysStatus::kMembershipCorrupted));
+  EXPECT_THAT(processed_response.downloaded_keys, IsEmpty());
 }
 
-TEST_P(DownloadKeysResponseHandlerTest, ShouldHandleRotationsToNewKeys) {
+TEST_F(DownloadKeysResponseHandlerTest, ShouldHandleRotationsToNewKeys) {
   const DownloadKeysResponseHandler::ProcessedResponse processed_response =
       handler().ProcessResponse(
           /*http_status=*/TrustedVaultRequest::HttpStatus::kSuccess,
Loading diff…

Original Bug Report

reported by vm...@google.com

Potential E2EE bypass in Trusted Vault via skipped rotation proof validation

Project Fortify, an experimental security project, has identified the following potential security issue.

Overview: A potential logic flaw in Chrome’s Trusted Vault implementation may allow a compromised server to bypass rotation proof verification and replace a client’s legitimate vault keys. By providing a crafted key with a version less than or equal to the client’s local version, the server can trigger a silent overwrite of the local keys. This could defeat end-to-end encryption for synced data like passwords, allowing the server to decrypt them.

Affected files:

  • components/trusted_vault/download_keys_response_handler.cc
  • components/trusted_vault/standalone_trusted_vault_backend.cc

Estimated timestamp from git blame: 2025-04-28

Description

There is a potential vulnerability in the Trusted Vault key handling code that could allow a compromised server (or a Man-in-the-Middle attacker capable of spoofing the server) to bypass rotation proof verification. This bypass allows the attacker to silently replace a user’s legitimate trusted vault keys with keys known to the attacker. If exploited, this breaks the end-to-end encryption (E2EE) guarantees of Chrome Sync, as future synced data will be encrypted using the attacker-controlled keys.

Note: The following analysis is based on code review by our setup. The steps represent a potential attack path, as a working proof-of-concept has not yet been executed or verified dynamically.

Technical Details

The vulnerability stems from a sequence of logic flaws across the key downloading and storage pipeline:

  1. Validation Bypass in IsValidKeyChain: In components/trusted_vault/download_keys_response_handler.cc, the IsValidKeyChain function iterates through downloaded keys to verify their rotation proofs. However, it explicitly skips verification for any keys with a version less than or equal to the client’s current version:

    // components/trusted_vault/download_keys_response_handler.cc:91
for (const ExtractedSharedKey& next_key : key_chain) {
  if (next_key.version <= last_valid_key_version) {
    continue; // Verification is skipped
  }
  // ... VerifyRotationProof() is called here ...
}

    If an attacker provides a key with version <= last_valid_key_version, the function returns true without verifying the key’s cryptographic linkage to the existing chain.

  2. Unvalidated Keys Returned on kNoNewKeys: In ProcessResponse, if the downloaded keys’ maximum version is less than or equal to the local version, the status is correctly set to kNoNewKeys. However, the function still returns the unvalidated, decrypted keys back to the caller:

    // components/trusted_vault/download_keys_response_handler.cc:228
if (extracted_keys->back().version <= last_trusted_vault_key_and_version_.version) {
  status = TrustedVaultDownloadKeysStatus::kNoNewKeys;
}
return ProcessedResponse(status, trusted_vault_keys, ...);

  3. Blind Overwrite in StandaloneTrustedVaultBackend: When StandaloneTrustedVaultBackend::OnKeysRecovered receives the kNoNewKeys status, it still calls StoreKeys with the provided (and unvalidated) keys, citing the need to purge old keys removed from the server:

    // components/trusted_vault/standalone_trusted_vault_backend.cc:770
case LocalRecoveryFactor::RecoveryStatus::kNoNewKeys: {
  StoreKeys(primary_account_->gaia, downloaded_vault_keys, last_vault_key_version);
  break;
}

    Finally, StoreKeys clears the user’s existing, legitimate vault keys and replaces them entirely with the attacker’s unvalidated keys.

Potential Attack Steps

To trigger this vulnerability, an attacker controlling the sync server would potentially perform the following steps:

  1. Wait for (or trigger via MarkLocalKeysAsStale) the client to attempt a key recovery/download via DownloadNewKeys.
  2. In response to the client’s request, construct a malicious SecurityDomainMember protobuf payload.
  3. Include a single SharedMemberKey in the payload, setting its epoch (version) to the client’s current version V (or lower).
  4. Set the wrapped_key to an attacker-generated key (K_evil), encrypted using the client’s public SecureBox key (which the server possesses from device registration).
  5. Omit a valid rotation_proof.
  6. The client decrypts K_evil, skips validation because its version is <= V, and processes it as a kNoNewKeys response.
  7. The client wipes its legitimate key K_legit and persists K_evil.
  8. The attacker can now passively decrypt any future E2EE sync data stored on the server.

Suggested Fix

There are two primary areas that should be patched to resolve this:

  1. Do not return unvalidated keys: In DownloadKeysResponseHandler::ProcessResponse, if the response results in kNoNewKeys (or if keys were skipped during validation), the handler should not include those unverified keys in the ProcessedResponse. It should return an empty list or only the keys that were strictly verified via VerifyRotationProof.
  2. Strict Equality/Validation Checking: Re-evaluate the logic in IsValidKeyChain. If older keys are supplied by the server to overwrite local storage, the client must verify that these older keys actually match the keys it already holds locally, rather than blindly skipping validation and trusting the server’s payload.

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
On This Page