From 52b54e757d9ddda84993f818e2a28a57680de94e Mon Sep 17 00:00:00 2001
From: Mikel Astiz <mastiz@chromium.org>
Date: Mon, 08 Jun 2026 01:43:38 -0700
Subject: [PATCH] [trusted vault] Prevent UAF in TrustedVaultEncryptionKeysExtension

This patch modifies SetSyncEncryptionKeys to parse arguments safely and
handle potential frame destruction during V8 object access.

Before this patch, SetSyncEncryptionKeys parsed the encryption_keys
argument using args->GetNext(&encryption_keys), which synchronously
executed user-defined JS getters on the array indices. If the getter
detached the frame, the extension object was destroyed, leading to a
Use-After-Free when execution resumed.

This patch fixes the issue by:
1. Extracting the encryption_keys argument as a raw
   v8::Local<v8::Value> first.
2. Parsing and converting the array elements to Mojo C++ structs in a
   helper function (ParseSyncEncryptionKeysMayDeleteFrame).
3. Binding the continuation (SetSyncEncryptionKeysContinue) using
   base::WeakPtr so it is not executed if the frame was destroyed
   during parsing.

This approach is consistent with the existing implementation of
SetClientEncryptionKeys.

Fixed: 513127137
Change-Id: I99542fa1dcf40766ad4b83b8dfb37357a4a4a24d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7900081
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1643030}
---

diff --git a/chrome/renderer/trusted_vault_encryption_keys_extension.cc b/chrome/renderer/trusted_vault_encryption_keys_extension.cc
index 8cf1f0a..9296f41 100644
--- a/chrome/renderer/trusted_vault_encryption_keys_extension.cc
+++ b/chrome/renderer/trusted_vault_encryption_keys_extension.cc
@@ -73,6 +73,26 @@
   return trusted_vault_keys;
 }
 
+// Parses an array of ArrayBuffers passed to `setSyncEncryptionKeys()`.
+// This method may run property callbacks during parsing, which could end up
+// deleting the frame.
+void ParseSyncEncryptionKeysMayDeleteFrame(
+    v8::Local<v8::Value> encryption_keys_value,
+    int32_t last_key_version,
+    base::OnceCallback<
+        void(std::optional<std::vector<chrome::mojom::TrustedVaultKeyPtr>>)>
+        callback) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  v8::LocalVector<v8::ArrayBuffer> encryption_keys(isolate);
+  if (!gin::ConvertFromV8(isolate, encryption_keys_value, &encryption_keys)) {
+    DVLOG(1) << "invalid encryption key";
+    std::move(callback).Run(std::nullopt);
+    return;
+  }
+  std::move(callback).Run(
+      SyncEncryptionKeysToTrustedVaultKeys(encryption_keys, last_key_version));
+}
+
 // Parses an array of key objects passed to `setClientEncryptionKeys()`.
 // The members of each object are `epoch` integer and `key` ArrayBuffer.
 bool ParseTrustedVaultKeyArrayMayDeleteFrame(
@@ -302,6 +322,10 @@
 
   v8::HandleScope handle_scope(args->isolate());
 
+  if (render_frame()->GetWebFrame()->MainWorldScriptContext().IsEmpty()) {
+    return;
+  }
+
   v8::Local<v8::Function> callback;
   if (!args->GetNext(&callback)) {
     RecordCallToSetSyncEncryptionKeysToUma(ValidArgs::kInvalidArgs);
@@ -318,17 +342,11 @@
     return;
   }
 
-  v8::LocalVector<v8::ArrayBuffer> encryption_keys(args->isolate());
-  if (!args->GetNext(&encryption_keys)) {
+  v8::Local<v8::Value> encryption_keys_value;
+  if (!args->GetNext(&encryption_keys_value) ||
+      !encryption_keys_value->IsArray()) {
     RecordCallToSetSyncEncryptionKeysToUma(ValidArgs::kInvalidArgs);
-    DLOG(ERROR) << "Not array of strings";
-    args->ThrowError();
-    return;
-  }
-
-  if (encryption_keys.empty()) {
-    RecordCallToSetSyncEncryptionKeysToUma(ValidArgs::kInvalidArgs);
-    DLOG(ERROR) << "Array of strings empty";
+    DLOG(ERROR) << "Not array";
     args->ThrowError();
     return;
   }
@@ -341,6 +359,34 @@
     return;
   }
 
+  ParseSyncEncryptionKeysMayDeleteFrame(
+      encryption_keys_value, last_key_version,
+      base::BindOnce(
+          &TrustedVaultEncryptionKeysExtension::SetSyncEncryptionKeysContinue,
+          weak_ptr_factory_.GetWeakPtr(), args, std::move(callback),
+          std::move(gaia_id)));
+}
+
+void TrustedVaultEncryptionKeysExtension::SetSyncEncryptionKeysContinue(
+    gin::Arguments* args,
+    v8::Local<v8::Function> callback,
+    std::string gaia_id,
+    std::optional<std::vector<chrome::mojom::TrustedVaultKeyPtr>>
+        encryption_keys) {
+  if (!encryption_keys) {
+    RecordCallToSetSyncEncryptionKeysToUma(ValidArgs::kInvalidArgs);
+    DLOG(ERROR) << "Can't parse encryption keys";
+    args->ThrowError();
+    return;
+  }
+
+  if (encryption_keys->empty()) {
+    RecordCallToSetSyncEncryptionKeysToUma(ValidArgs::kInvalidArgs);
+    DLOG(ERROR) << "Array of strings empty";
+    args->ThrowError();
+    return;
+  }
+
   auto global_callback =
       std::make_unique<v8::Global<v8::Function>>(args->isolate(), callback);
 
@@ -353,9 +399,8 @@
   std::vector<
       std::pair<std::string, std::vector<chrome::mojom::TrustedVaultKeyPtr>>>
       trusted_vault_keys;
-  trusted_vault_keys.emplace_back(
-      trusted_vault::kSyncSecurityDomainName,
-      SyncEncryptionKeysToTrustedVaultKeys(encryption_keys, last_key_version));
+  trusted_vault_keys.emplace_back(trusted_vault::kSyncSecurityDomainName,
+                                  *std::move(encryption_keys));
   remote_->SetEncryptionKeys(
       std::move(gaia_id), std::move(trusted_vault_keys),
       base::BindOnce(
diff --git a/chrome/renderer/trusted_vault_encryption_keys_extension.h b/chrome/renderer/trusted_vault_encryption_keys_extension.h
index 123e1d2..bb742fb 100644
--- a/chrome/renderer/trusted_vault_encryption_keys_extension.h
+++ b/chrome/renderer/trusted_vault_encryption_keys_extension.h
@@ -6,6 +6,7 @@
 #define CHROME_RENDERER_TRUSTED_VAULT_ENCRYPTION_KEYS_EXTENSION_H_
 
 #include <memory>
+#include <vector>
 
 #include "base/memory/weak_ptr.h"
 #include "chrome/common/trusted_vault_encryption_keys_extension.mojom.h"
@@ -45,6 +46,12 @@
   void Install();
 #if !BUILDFLAG(IS_ANDROID)
   void SetSyncEncryptionKeys(gin::Arguments* args);
+  void SetSyncEncryptionKeysContinue(
+      gin::Arguments* args,
+      v8::Local<v8::Function> callback,
+      std::string gaia_id,
+      std::optional<std::vector<chrome::mojom::TrustedVaultKeyPtr>>
+          encryption_keys);
   void SetClientEncryptionKeys(gin::Arguments* args);
   void SetClientEncryptionKeysContinue(
       gin::Arguments* args,