Chrome · Password Manager
CVE-2026-11084
Logic Error in Password Manager
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifchrome/browser/password_manager/password_change/change_password_form_waiter.cc |
modified |
Files Changed
chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.ccchrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.hchrome/browser/password_manager/password_change/change_password_form_filling_submission_helper_unittest.ccchrome/browser/password_manager/password_change/change_password_form_waiter.cc
Patch
From e7f06a9ab599fb35618a5efebd1c78ff331d60e2 Mon Sep 17 00:00:00 2001 From: Viktor Semeniuk <[email protected]> Date: Fri, 10 Apr 2026 09:02:15 -0700 Subject: [PATCH] Use FieldGlobalId instead of FieldRendererId Fixed: 500124500 Change-Id: Ib194079c4533c4f27f689a8daa06d760806ff614 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7735801 Reviewed-by: Vasilii Sukhanov <[email protected]> Commit-Queue: Viktor Semeniuk <[email protected]> Cr-Commit-Position: refs/heads/main@{#1612904} --- diff --git a/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.cc b/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.cc index 84732c5..51782f7 100644 --- a/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.cc +++ b/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.cc @@ -199,7 +199,8 @@ return; } - observed_fields_.push_back(form.new_password_element_renderer_id); + observed_fields_.push_back(autofill::FieldGlobalId{ + form.form_data.host_frame(), form.new_password_element_renderer_id}); if (auto logger = GetLoggerIfAvailable(client_)) { logger->LogString( diff --git a/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.h b/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.h index a8b071cd..b9a0e88 100644 --- a/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.h +++ b/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.h @@ -16,6 +16,7 @@ #include "chrome/browser/password_manager/password_change/password_change_submission_verifier.h" #include "chrome/common/chrome_render_frame.mojom.h" #include "components/autofill/core/common/form_data.h" +#include "components/autofill/core/common/unique_ids.h" #include "components/optimization_guide/content/browser/page_content_proto_provider.h" #include "components/password_manager/core/browser/password_form.h" @@ -165,9 +166,9 @@ std::unique_ptr<ButtonClickHelper> click_helper_; - // new_password_element_renderer_ids for the forms which `this` tried to fill. + // FieldGlobalIds for the forms which `this` tried to fill. // Used to avoid attempting to fill the same form over and over again. - std::vector<autofill::FieldRendererId> observed_fields_; + std::vector<autofill::FieldGlobalId> observed_fields_; // Helper object which finds for a new PasswordFormManager when filling of an // old form failed. diff --git a/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper_unittest.cc b/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper_unittest.cc index 4ca16e57..ad6d81ef 100644 --- a/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper_unittest.cc +++ b/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper_unittest.cc @@ -545,15 +545,20 @@ EXPECT_TRUE(verifier->form_waiter()); - auto* new_form_manager = CreateFormManagerFromFormData( - CreateTestPasswordFormData("", "", 101, 102), /*credentials_to_seed=*/{}); + autofill::FormData new_form_data = + CreateTestPasswordFormData("", "", 101, 102); + new_form_data.set_renderer_id(autofill::test::MakeFormRendererId()); + auto* new_form_manager = + CreateFormManagerFromFormData(new_form_data, /*credentials_to_seed=*/{}); // Verify that Chrome attempts to fill and submit a newly found form. + autofill::FormData filled_form = CreateFilledTestPasswordFormData(); + filled_form.set_renderer_id(new_form_data.renderer_id()); + if (!base::FeatureList::IsEnabled( password_manager::features::kFillChangePasswordFormByTyping)) { EXPECT_CALL(driver(), FillChangePasswordForm) - .WillOnce( - base::test::RunOnceCallback<5>(CreateFilledTestPasswordFormData())); + .WillOnce(base::test::RunOnceCallback<5>(filled_form)); } static_cast<password_manager::PasswordFormManagerObserver*>( verifier->form_waiter()) @@ -562,8 +567,7 @@ if (base::FeatureList::IsEnabled( password_manager::features::kFillChangePasswordFormByTyping)) { ASSERT_TRUE(verifier->form_filler()); - verifier->form_filler()->SimulateFillingResult( - CreateFilledTestPasswordFormData()); + verifier->form_filler()->SimulateFillingResult(filled_form); } else { EXPECT_TRUE(base::test::RunUntil( [&]() { return verifier->capturer() != nullptr; })); @@ -657,14 +661,19 @@ PasswordChangeQuality_StepQuality_SubmissionStatus_FORM_FILLING_FAILED); EXPECT_TRUE(verifier->form_waiter()); - auto* new_form_manager = CreateFormManagerFromFormData( - CreateTestPasswordFormData("", "", 101, 102), /*credentials_to_seed=*/{}); + autofill::FormData new_form_data = + CreateTestPasswordFormData("", "", 101, 102); + new_form_data.set_renderer_id(autofill::test::MakeFormRendererId()); + auto* new_form_manager = + CreateFormManagerFromFormData(new_form_data, /*credentials_to_seed=*/{}); + + autofill::FormData filled_form = CreateFilledTestPasswordFormData(); + filled_form.set_renderer_id(new_form_data.renderer_id()); if (!base::FeatureList::IsEnabled( password_manager::features::kFillChangePasswordFormByTyping)) { EXPECT_CALL(driver(), FillChangePasswordForm) - .WillOnce( - base::test::RunOnceCallback<5>(CreateFilledTestPasswordFormData())); + .WillOnce(base::test::RunOnceCallback<5>(filled_form)); } static_cast<password_manager::PasswordFormManagerObserver*>( verifier->form_waiter()) @@ -673,8 +682,7 @@ if (base::FeatureList::IsEnabled( password_manager::features::kFillChangePasswordFormByTyping)) { ASSERT_TRUE(verifier->form_filler()); - verifier->form_filler()->SimulateFillingResult( - CreateFilledTestPasswordFormData()); + verifier->form_filler()->SimulateFillingResult(filled_form); } else { EXPECT_TRUE(base::test::RunUntil( [&]() { return verifier->capturer() != nullptr; })); diff --git a/chrome/browser/password_manager/password_change/change_password_form_waiter.cc b/chrome/browser/password_manager/password_change/change_password_form_waiter.cc index a7aafa5b..0f8399a 100644 --- a/chrome/browser/password_manager/password_change/change_password_form_waiter.cc +++ b/chrome/browser/password_manager/password_change/change_password_form_waiter.cc @@ -9,6 +9,7 @@ #include "base/task/single_thread_task_runner.h" #include "components/autofill/content/browser/content_autofill_client.h" #include "components/autofill/core/browser/ml_model/field_classification_model_handler.h" +#include "components/autofill/core/common/unique_ids.h" #include "components/password_manager/core/browser/features/password_features.h" #include "components/password_manager/core/browser/password_form.h" #include "components/password_manager/core/browser/password_form_manager.h" @@ -107,7 +108,7 @@ ChangePasswordFormWaiter::Builder& ChangePasswordFormWaiter::Builder::SetFieldsToIgnore( - const std::vector<autofill::FieldRendererId>& fields_to_ignore) { + const std::vector<autofill::FieldGlobalId>& fields_to_ignore) { form_waiter_->fields_to_ignore_ = fields_to_ignore; return *this; } @@ -152,7 +153,10 @@ auto callback = base::BindOnce( &ChangePasswordFormWaiter::GetCorrespondingFormManager, weak_ptr_factory_.GetWeakPtr(), - manager->GetParsedObservedForm()->new_password_element_renderer_id); + autofill::FieldGlobalId{ + manager->GetParsedObservedForm()->form_data.host_frame(), + manager->GetParsedObservedForm() + ->new_password_element_renderer_id}); // The form has been already parsed. Invoke OnPasswordFormParsed to check // if the form is eligible. @@ -198,9 +202,12 @@ return; } - if (std::ranges::count(fields_to_ignore_, - form_manager->GetParsedObservedForm() - ->new_password_element_renderer_id)) { + if (std::ranges::count( + fields_to_ignore_, + autofill::FieldGlobalId{ + form_manager->GetParsedObservedForm()->form_data.host_frame(), + form_manager->GetParsedObservedForm() + ->new_password_element_renderer_id})) { return; } @@ -210,22 +217,25 @@ auto new_field_id = form_manager->GetParsedObservedForm()->new_password_element_renderer_id; + auto field_global_id = autofill::FieldGlobalId{ + form_manager->GetParsedObservedForm()->form_data.host_frame(), + new_field_id}; form_manager->GetDriver()->CheckViewAreaVisible( new_field_id, base::BindOnce(&ChangePasswordFormWaiter::OnCheckViewAreaVisibleCallback, - weak_ptr_factory_.GetWeakPtr(), new_field_id)); + weak_ptr_factory_.GetWeakPtr(), field_global_id)); return; } void ChangePasswordFormWaiter::OnCheckViewAreaVisibleCallback( - autofill::FieldRendererId new_password_element_id, + autofill::FieldGlobalId field_global_id, bool is_visible) { if (!is_visible) { return; } if (auto* form_manager = GetCorrespondingFormManager( - weak_ptr_factory_.GetWeakPtr(), new_password_element_id)) { + weak_ptr_factory_.GetWeakPtr(), field_global_id)) { std::move(callback_).Run(form_manager);
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper_unittest.cc b/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper_unittest.cc
index 4ca16e57..ad6d81ef 100644
--- a/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper_unittest.cc
+++ b/chrome/browser/password_manager/password_change/change_password_form_filling_submission_helper_unittest.cc
@@ -545,15 +545,20 @@
EXPECT_TRUE(verifier->form_waiter());
- auto* new_form_manager = CreateFormManagerFromFormData(
- CreateTestPasswordFormData("", "", 101, 102), /*credentials_to_seed=*/{});
+ autofill::FormData new_form_data =
+ CreateTestPasswordFormData("", "", 101, 102);
+ new_form_data.set_renderer_id(autofill::test::MakeFormRendererId());
+ auto* new_form_manager =
+ CreateFormManagerFromFormData(new_form_data, /*credentials_to_seed=*/{});
// Verify that Chrome attempts to fill and submit a newly found form.
+ autofill::FormData filled_form = CreateFilledTestPasswordFormData();
+ filled_form.set_renderer_id(new_form_data.renderer_id());
+
if (!base::FeatureList::IsEnabled(
password_manager::features::kFillChangePasswordFormByTyping)) {
EXPECT_CALL(driver(), FillChangePasswordForm)
- .WillOnce(
- base::test::RunOnceCallback<5>(CreateFilledTestPasswordFormData()));
+ .WillOnce(base::test::RunOnceCallback<5>(filled_form));
}
static_cast<password_manager::PasswordFormManagerObserver*>(
verifier->form_waiter())
@@ -562,8 +567,7 @@
if (base::FeatureList::IsEnabled(
password_manager::features::kFillChangePasswordFormByTyping)) {
ASSERT_TRUE(verifier->form_filler());
- verifier->form_filler()->SimulateFillingResult(
- CreateFilledTestPasswordFormData());
+ verifier->form_filler()->SimulateFillingResult(filled_form);
} else {
EXPECT_TRUE(base::test::RunUntil(
[&]() { return verifier->capturer() != nullptr; }));
@@ -657,14 +661,19 @@
PasswordChangeQuality_StepQuality_SubmissionStatus_FORM_FILLING_FAILED);
EXPECT_TRUE(verifier->form_waiter());
- auto* new_form_manager = CreateFormManagerFromFormData(
- CreateTestPasswordFormData("", "", 101, 102), /*credentials_to_seed=*/{});
+ autofill::FormData new_form_data =
+ CreateTestPasswordFormData("", "", 101, 102);
+ new_form_data.set_renderer_id(autofill::test::MakeFormRendererId());
+ auto* new_form_manager =
+ CreateFormManagerFromFormData(new_form_data, /*credentials_to_seed=*/{});
+
+ autofill::FormData filled_form = CreateFilledTestPasswordFormData();
+ filled_form.set_renderer_id(new_form_data.renderer_id());
if (!base::FeatureList::IsEnabled(
password_manager::features::kFillChangePasswordFormByTyping)) {
EXPECT_CALL(driver(), FillChangePasswordForm)
- .WillOnce(
- base::test::RunOnceCallback<5>(CreateFilledTestPasswordFormData()));
+ .WillOnce(base::test::RunOnceCallback<5>(filled_form));
}
static_cast<password_manager::PasswordFormManagerObserver*>(
verifier->form_waiter())
@@ -673,8 +682,7 @@
if (base::FeatureList::IsEnabled(
password_manager::features::kFillChangePasswordFormByTyping)) {
ASSERT_TRUE(verifier->form_filler());
- verifier->form_filler()->SimulateFillingResult(
- CreateFilledTestPasswordFormData());
+ verifier->form_filler()->SimulateFillingResult(filled_form);
} else {
EXPECT_TRUE(base::test::RunUntil(
[&]() { return verifier->capturer() != nullptr; }));
diff --git a/chrome/browser/password_manager/password_change/change_password_form_waiter_unittest.cc b/chrome/browser/password_manager/password_change/change_password_form_waiter_unittest.cc
index 1a7a4fab..617e201 100644
--- a/chrome/browser/password_manager/password_change/change_password_form_waiter_unittest.cc
+++ b/chrome/browser/password_manager/password_change/change_password_form_waiter_unittest.cc
@@ -16,6 +16,7 @@
#include "components/autofill/content/browser/test_content_autofill_client.h"
#include "components/autofill/core/browser/ml_model/field_classification_model_handler.h"
#include "components/autofill/core/common/autofill_test_utils.h"
+#include "components/autofill/core/common/unique_ids.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/optimization_guide/core/delivery/test_optimization_guide_model_provider.h"
#include "components/os_crypt/sync/os_crypt_mocker.h"
@@ -413,9 +414,12 @@
parsed_form->new_password_element_renderer_id;
ASSERT_EQ(new_password_renderer_id, autofill::FieldRendererId(2));
+ autofill::FieldGlobalId field_global_id = {
+ parsed_form->form_data.host_frame(),
+ parsed_form->new_password_element_renderer_id};
auto waiter = ChangePasswordFormWaiter::Builder(web_contents(), client(),
completion_callback.Get())
- .SetFieldsToIgnore({new_password_renderer_id})
+ .SetFieldsToIgnore({field_global_id})
.SetTimeoutCallback(timeout_callback.Get())
.Build();
Loading diff…
Original Bug Report
reported by [email protected]
Cross-frame Password Leak in ChangePasswordFormWaiter via FieldRendererId Collision
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 without the security team.
Overview: The ChangePasswordFormWaiter incorrectly looks up forms using a FieldRendererId, which is not globally unique across frames. An attacker in a cross-origin iframe can spray predictable IDs to cause a collision, causing the user’s plaintext password to be filled into the attacker’s iframe during an Automated Password Change flow.
Affected files:
chrome/browser/password_manager/password_change/change_password_form_waiter.ccchrome/browser/password_manager/password_change_delegate_impl.ccchrome/browser/password_manager/password_change/password_change_from_checkup_delegate.ccchrome/browser/password_manager/password_change/change_password_form_filling_submission_helper.cc
Estimated timestamp from git blame: 2025-10-14
Description
A potential security vulnerability exists in the Automated Password Change flow due to a cross-frame confusion issue. The class ChangePasswordFormWaiter relies on autofill::FieldRendererId to uniquely identify a PasswordFormManager across an entire tab. However, FieldRendererId (which wraps a Blink DOMNodeId) is generated using a per-renderer-process counter that starts at 1. It is not globally unique across different frames, especially cross-origin iframes isolated via Site Isolation.
In chrome/browser/password_manager/password_change/change_password_form_waiter.cc, the static function GetCorrespondingFormManager searches the PasswordFormCache (which stores form managers for the entire WebContents) using only the FieldRendererId. It returns the first matching manager it finds:
for (const auto& manager : cache->GetFormManagers()) {
if (manager->GetParsedObservedForm() &&
manager->GetParsedObservedForm()->new_password_element_renderer_id ==
new_password_element_id) {
return manager.get();
}
}
Because the lookup returns the first match and ignores the frame/driver, an attacker can exploit this by ensuring a malicious form manager is placed earlier in the cache with a colliding ID.
Potential Exploitation Steps
- Setup: A user initiates an Automated Password Change flow on a legitimate site (e.g.,
victim.example/change-password). - Attacker Iframe: The legitimate site embeds a cross-origin attacker-controlled iframe (e.g., via a compromised ad or widget).
- ID Spraying: The attacker’s iframe dynamically creates a large number (e.g., 100) of hidden forms containing
<input autocomplete="current-password">and<input autocomplete="new-password">elements. Because the attacker’s renderer process has its own counter starting at 1, thesenew-passwordelements receiveFieldRendererIds from 1 to 100. - Cache Population: These forms are parsed and their
PasswordFormManagers are added to the browser’s per-WebContentsPasswordFormCache. - Legitimate Form: The legitimate main frame’s form is parsed. It receives an ID from its own process’s counter (e.g.,
42). It is added to the end of the cache. - Visibility Check Race:
ChangePasswordFormWaiterinitiates visibility checks for all forms in the cache. The attacker’s 100 hidden forms fail the visibility check and their callbacks return early. - Legitimate Form Visible: The legitimate form passes the visibility check.
OnCheckViewAreaVisibleCallbackis invoked with ID42. - Vulnerable Lookup: The callback calls
GetCorrespondingFormManager(42). The function scans the cache and returns the attacker’s form manager (which was added earlier and also has ID42), ignoring the legitimate form at the end of the list. - Password Leak: The flow proceeds to
ChangePasswordFormFillingSubmissionHelper::FillChangePasswordFormwithout validating the origin. It triggers an IPC that sends the user’s current, plaintext password (and a newly generated password) directly to the attacker’s renderer process to be autofilled into the hidden form. - Theft: The attacker’s JavaScript reads the values from the hidden form, stealing the credentials.
(Note: These are suggested steps based on static analysis; our tooling has not run a live proof of concept.)
Suggested Fix
Do not rely solely on FieldRendererId for lookups across a WebContents. Security logic should use FieldGlobalId (which combines the FieldRendererId with a LocalFrameToken), as explicitly warned in components/autofill/core/common/unique_ids.h.
Alternatively, GetCorrespondingFormManager should be updated to accept and require the expected PasswordManagerDriver (or its associated LocalFrameToken), and the loop should verify that manager->GetDriver() == expected_driver alongside the renderer ID.
Evaluated with Chrome root at commit: f200f57a19490707ff8bc7aa5de3cbc443a3afad
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. And please feel free to reach out to me directly if you have concerns or feedback on the project.
References
On This Page