CVE-2026-7921
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifchrome/browser/ui/passwords/manage_passwords_state.cc |
modified | |
MockPasswordFormManagerForUIcomponents/password_manager/core/browser/mock_password_form_manager_for_ui.cc |
modified |
Files Changed
chrome/browser/ui/passwords/manage_passwords_state.ccchrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cccomponents/password_manager/core/browser/http_auth_manager_impl.cccomponents/password_manager/core/browser/mock_password_form_manager_for_ui.cccomponents/password_manager/core/browser/mock_password_form_manager_for_ui.hcomponents/password_manager/core/browser/password_form_manager.cccomponents/password_manager/core/browser/password_form_manager.hcomponents/password_manager/core/browser/password_form_manager_for_ui.hcomponents/password_manager/core/browser/password_generation_manager.cccomponents/password_manager/core/browser/password_manager.cc
Patch
From c54219453bed4f9e1c1e7c7e8b17567c8e350d4d Mon Sep 17 00:00:00 2001 From: Vasilii Sukhanov <[email protected]> Date: Wed, 08 Apr 2026 07:02:32 -0700 Subject: [PATCH] Fix UAF in PasswordFormManager::OnFetchCompleted Ensure that PasswordFormManager is not destroyed synchronously during OnFetchCompleted by checking if credentials are fetched before showing the manual fallback UI. Also added CHECKs in ManagePasswordsState to ensure that it only receives managers that have completed their fetch. Fixed: 499062376 Change-Id: Id87951491f42b6bc748185049898a58e7287fc6f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7737082 Commit-Queue: Vasilii Sukhanov <[email protected]> Reviewed-by: Rafał Godlewski <[email protected]> Cr-Commit-Position: refs/heads/main@{#1611458} --- diff --git a/chrome/browser/ui/passwords/manage_passwords_state.cc b/chrome/browser/ui/passwords/manage_passwords_state.cc index 55ebb0c..8ee0251 100644 --- a/chrome/browser/ui/passwords/manage_passwords_state.cc +++ b/chrome/browser/ui/passwords/manage_passwords_state.cc @@ -84,6 +84,7 @@ void ManagePasswordsState::OnPendingPassword( std::unique_ptr<PasswordFormManagerForUI> form_manager) { + CHECK(form_manager->IsFetchCompleted()); ClearData(); form_manager_ = std::move(form_manager); local_credentials_forms_ = @@ -96,6 +97,7 @@ void ManagePasswordsState::OnUpdatePassword( std::unique_ptr<password_manager::PasswordFormManagerForUI> form_manager) { + CHECK(form_manager->IsFetchCompleted()); ClearData(); form_manager_ = std::move(form_manager); local_credentials_forms_ = @@ -127,6 +129,7 @@ void ManagePasswordsState::OnAutomaticPasswordSave( std::unique_ptr<PasswordFormManagerForUI> form_manager) { + CHECK(form_manager->IsFetchCompleted()); ClearData(); form_manager_ = std::move(form_manager); local_credentials_forms_ = @@ -145,6 +148,7 @@ state == password_manager::ui::UPDATE_CONFIRMATION_STATE || state == password_manager::ui::GENERATED_PASSWORD_CONFIRMATION_STATE); if (form_manager) { + CHECK(form_manager->IsFetchCompleted()); ClearData(); form_manager_ = std::move(form_manager); } @@ -215,6 +219,7 @@ void ManagePasswordsState::OnPasswordMovable( std::unique_ptr<PasswordFormManagerForUI> form_to_move) { + CHECK(form_to_move->IsFetchCompleted()); ClearData(); form_manager_ = std::move(form_to_move); local_credentials_forms_ = diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc index b6db2e2..36d1de7 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc @@ -258,6 +258,8 @@ .Times(AtMost(2)) .WillRepeatedly( Return(base::span<const password_manager::PasswordForm>())); + EXPECT_CALL(*form_manager, IsFetchCompleted()) + .WillRepeatedly(testing::Return(true)); EXPECT_CALL(*form_manager, GetURL()) .Times(AtMost(2)) .WillRepeatedly(ReturnRef(password_form->url)); diff --git a/components/password_manager/core/browser/http_auth_manager_impl.cc b/components/password_manager/core/browser/http_auth_manager_impl.cc index f691155..f81ab875 100644 --- a/components/password_manager/core/browser/http_auth_manager_impl.cc +++ b/components/password_manager/core/browser/http_auth_manager_impl.cc @@ -184,8 +184,7 @@ return; } - if (form_manager_->GetFormFetcher()->GetState() == - FormFetcher::State::WAITING) { + if (!form_manager_->IsFetchCompleted()) { // We have a provisional save manager, but it didn't finish matching yet. // We just give up. return; diff --git a/components/password_manager/core/browser/mock_password_form_manager_for_ui.cc b/components/password_manager/core/browser/mock_password_form_manager_for_ui.cc index 4eefe7d..9d4835f 100644 --- a/components/password_manager/core/browser/mock_password_form_manager_for_ui.cc +++ b/components/password_manager/core/browser/mock_password_form_manager_for_ui.cc @@ -6,7 +6,9 @@ namespace password_manager { -MockPasswordFormManagerForUI::MockPasswordFormManagerForUI() = default; +MockPasswordFormManagerForUI::MockPasswordFormManagerForUI() { + ON_CALL(*this, IsFetchCompleted()).WillByDefault(testing::Return(true)); +} MockPasswordFormManagerForUI::~MockPasswordFormManagerForUI() = default; } // namespace password_manager diff --git a/components/password_manager/core/browser/mock_password_form_manager_for_ui.h b/components/password_manager/core/browser/mock_password_form_manager_for_ui.h index 088c28a..526e400f 100644 --- a/components/password_manager/core/browser/mock_password_form_manager_for_ui.h +++ b/components/password_manager/core/browser/mock_password_form_manager_for_ui.h @@ -44,6 +44,7 @@ (), (const override)); MOCK_METHOD(bool, IsBlocklisted, (), (const override)); + MOCK_METHOD(bool, IsFetchCompleted, (), (const override)); MOCK_METHOD(bool, IsMovableToAccountStore, (), (const override)); MOCK_METHOD(void, Save, (), (override)); MOCK_METHOD(bool, diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc index ea1c0ca8..0aa08a1 100644 --- a/components/password_manager/core/browser/password_form_manager.cc +++ b/components/password_manager/core/browser/password_form_manager.cc @@ -462,6 +462,10 @@ return form_fetcher_->IsBlocklisted() || newly_blocklisted_; } +bool PasswordFormManager::IsFetchCompleted() const { + return form_fetcher_->GetState() != FormFetcher::State::WAITING; +} + bool PasswordFormManager::IsMovableToAccountStore() const { if (!client_->GetPasswordFeatureManager()->IsAccountStorageActive()) { return false; diff --git a/components/password_manager/core/browser/password_form_manager.h b/components/password_manager/core/browser/password_form_manager.h index ec69629..aa8af91 100644 --- a/components/password_manager/core/browser/password_form_manager.h +++ b/components/password_manager/core/browser/password_form_manager.h @@ -200,6 +200,7 @@ base::span<const InteractionsStats> GetInteractionsStats() const override; base::span<const PasswordForm> GetInsecureCredentials() const override; bool IsBlocklisted() const override; + bool IsFetchCompleted() const override; bool IsMovableToAccountStore() const override; void Save() override; diff --git a/components/password_manager/core/browser/password_form_manager_for_ui.h b/components/password_manager/core/browser/password_form_manager_for_ui.h index 2c650222..1064240a 100644 --- a/components/password_manager/core/browser/password_form_manager_for_ui.h +++ b/components/password_manager/core/browser/password_form_manager_for_ui.h @@ -57,6 +57,9 @@ // Determines if the user opted to 'never remember' passwords for this form. virtual bool IsBlocklisted() const = 0; + // Returns true if the fetch of credentials from the store is completed. + virtual bool IsFetchCompleted() const = 0; + // Determines whether the submitted credentials returned by // GetPendingCredentials() can be moved to the signed in account store. // Returns true if the submitted credentials are stored in the profile store diff --git a/components/password_manager/core/browser/password_generation_manager.cc b/components/password_manager/core/browser/password_generation_manager.cc index 8e9ca52..dad4c68 100644 --- a/components/password_manager/core/browser/password_generation_manager.cc +++ b/components/password_manager/core/browser/password_generation_manager.cc @@ -62,6 +62,7 @@ base::span<const InteractionsStats> GetInteractionsStats() const override; base::span<const PasswordForm> GetInsecureCredentials() const override; bool IsBlocklisted() const override; + bool IsFetchCompleted() const override; bool IsMovableToAccountStore() const override; void Save() override; bool IsUpdateAffectingPasswordsStoredInTheGoogleAccount() const override; @@ -148,6 +149,10 @@ return false; } +bool PasswordDataForUI::IsFetchCompleted() const { + return true; +} + bool PasswordDataForUI::IsMovableToAccountStore() const { // This is irrelevant for the generation conflict resolution bubble. return false; diff --git a/components/password_manager/core/browser/password_manager.cc b/components/password_manager/core/browser/password_manager.cc index 5a9f0fd..7d3729b 100644 --- a/components/password_manager/core/browser/password_manager.cc +++ b/components/password_manager/core/browser/password_manager.cc @@ -1044,8 +1044,7 @@ } for (const auto& form_manager : password_form_cache_.GetFormManagers()) { if (form_manager->GetDriver().get() == driver && - form_manager->GetFormFetcher()->GetState() == - FormFetcher::State::WAITING) { + !form_manager->IsFetchCompleted()) { return false; } } @@ -1216,8 +1215,7 @@ } } - if (is_manual_fallback && matched_manager->GetFormFetcher()->GetState() == - FormFetcher::State::WAITING) {
Regression Test / PoC
diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
index b6db2e2..36d1de7 100644
--- a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
+++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
@@ -258,6 +258,8 @@
.Times(AtMost(2))
.WillRepeatedly(
Return(base::span<const password_manager::PasswordForm>()));
+ EXPECT_CALL(*form_manager, IsFetchCompleted())
+ .WillRepeatedly(testing::Return(true));
EXPECT_CALL(*form_manager, GetURL())
.Times(AtMost(2))
.WillRepeatedly(ReturnRef(password_form->url));
diff --git a/components/password_manager/core/browser/password_manager_unittest.cc b/components/password_manager/core/browser/password_manager_unittest.cc
index bd6c456..339864a6 100644
--- a/components/password_manager/core/browser/password_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_manager_unittest.cc
@@ -1535,6 +1535,26 @@
user_action_tester.GetActionCount("PasswordManager_LoginPassed"));
}
+TEST_P(PasswordManagerTest, NoManualFallbackWhenFetchIsPending) {
+ EXPECT_CALL(client_, IsSavingAndFillingEnabled).WillRepeatedly(Return(true));
+
+ FormData form_data = MakeSimpleFormData();
+ std::vector<FormData> observed = {form_data};
+
+ // Register found form in PasswordManager. This starts the fetch.
+ manager()->OnPasswordFormsParsed(&driver_, observed);
+
+ // Do NOT call task_environment_.RunUntilIdle() here to keep fetch pending.
+
+ // The user types a password. Fallback should NOT be shown because fetch is
+ // pending.
+ EXPECT_CALL(client_, ShowManualFallbackForSaving).Times(0);
+
+ FormData user_input_form = form_data;
+ test_api(user_input_form).field(1).set_value(u"password");
+ manager()->OnInformAboutUserInput(&driver_, user_input_form);
+}
+
TEST_P(PasswordManagerTest, DoNotSaveWhenUserDeletesPassword) {
PasswordForm form(MakeSimpleForm());
PasswordForm stored_form = form;
Original Bug Report
Browser Process UAF in PasswordFormManager::OnFetchCompleted
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: A potential Use-After-Free (UAF) vulnerability exists in the browser process due to synchronous object destruction within PasswordFormManager::OnFetchCompleted. An attacker can manipulate UI states to free the object while its method is still executing, leading to an exploitable UAF virtual call. This could potentially result in Remote Code Execution (RCE) and a sandbox escape.
Affected files:
components/password_manager/core/browser/password_form_manager.cc
Estimated timestamp from git blame: 2024-02-09
Description
A potential Use-After-Free (UAF) vulnerability exists in components/password_manager/core/browser/password_form_manager.cc within the browser process.
The function PasswordFormManager::OnFetchCompleted() contains a critical flaw where it calls FillNow() and subsequently accesses its own member variables (should_schedule_save_for_later_) and methods (Save()). The FillNow() method explicitly warns developers that it can trigger the synchronous destruction of the PasswordFormManager instance. If destruction occurs, the execution returning to OnFetchCompleted() will operate on freed memory.
void PasswordFormManager::OnFetchCompleted() {
// ...
if (...) {
FillNow(); // Can synchronously destroy `this`
}
// UAF Read
if (should_schedule_save_for_later_) {
should_schedule_save_for_later_ = false;
Save(); // UAF Call -> triggers virtual call on raw_ptr
}
}
Root Cause Analysis
The PasswordFormManager can be synchronously destroyed if FillNow() leads to an autofill UI notification while the UI is in a specific state.
When FillNow() executes, it calls SendFillInformationToRenderer(). If previous credentials exist for the site, this triggers ManagePasswordsUIController::OnPasswordAutofilled(). Normally, this controller ignores autofill events if it is displaying a save prompt (PENDING_PASSWORD_STATE or SAVE_CONFIRMATION_STATE). However, if the user hides the save bubble, the UI transitions to MANAGE_STATE without freeing the PasswordFormManager.
If the delayed fetch completes while in MANAGE_STATE, ManagePasswordsUIController accepts the autofill event and calls ManagePasswordsState::ClearData(). This function executes form_manager_.reset(), synchronously deleting the executing PasswordFormManager.
Because the object is managed by a std::unique_ptr and has no active raw_ptr references to it globally, MiraclePtr (BackupRefPtr) does not quarantine the allocation. The this pointer on the stack becomes dangling.
Suggested Potential Steps to Reproduce
Note: These are suggested/potential steps derived from code analysis. Our tooling agent does not yet have the ability to run code, so a working Proof of Concept has not been executed.
- Precondition: The user visits an attacker-controlled site where they have previously saved credentials.
- Generate Password: The user interacts with a registration form and uses Chrome’s built-in password generator. Chrome creates a
PasswordFormManager, begins fetching stored credentials, and shows the “Save Password” confirmation bubble. - Delay and Hide: The attacker artificially delays the completion of the credential fetch. The user hides the save bubble (e.g., by clicking elsewhere or switching tabs), transitioning the UI to
MANAGE_STATEwhile the fetch is still pending. - Trigger UAF: The attacker releases the delay, completing the fetch.
OnFetchCompleted()fires and callsFillNow(). Because the UI is inMANAGE_STATE, the resulting autofill notification triggersManagePasswordsState::ClearData(), destroying thePasswordFormManager. - Exploitation: Execution unwinds to
OnFetchCompleted(). The attacker grooms the heap to reallocate the freed object, setting theshould_schedule_save_for_later_byte totrue.OnFetchCompletedcallsSave(), which attempts to invoke the virtual methodform_fetcher_->GetState(). The attacker overwrites theform_fetcher_pointer in the reallocated memory, hijacking the virtual call to achieve Remote Code Execution (RCE) in the browser process.
Suggested Fix
To prevent this UAF, PasswordFormManager should not execute any logic after calling FillNow().
The safest mitigation is to use a base::WeakPtr to track the object’s lifetime across the FillNow() call:
void PasswordFormManager::OnFetchCompleted() {
// ...
base::WeakPtr<PasswordFormManager> weak_this = weak_ptr_factory_.GetWeakPtr();
if (...) {
FillNow();
}
if (!weak_this) {
return; // Object was destroyed during FillNow()
}
if (should_schedule_save_for_later_) {
should_schedule_save_for_later_ = false;
Save();
}
}
Alternatively, investigate if Save() can be scheduled asynchronously or if the should_schedule_save_for_later_ logic can be moved prior to FillNow().
Evaluated with Chrome root at commit: ff3d2b74fa39431785bd60e51463b08fcc71ee33
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.