CVE-2026-7921
Overview
Background
- `PasswordFormManager`
- the browser-side object that owns the state of a single observed login form, including the pending credentials and a background fetch of matching stored passwords.
- `FormFetcher`
- the helper that asynchronously queries the password store; it sits in
FormFetcher::State::WAITINGuntil the query resolves, after which it drivesOnFetchCompleted. - `ManagePasswordsState`
- the UI-layer state holder that takes ownership of a
PasswordFormManagerForUI(viastd::move) when a save/update/move bubble or manual fallback is shown. - Manual fallback for saving
- the UI offered when the user types into a password field, which can be triggered before the store fetch for that form has finished.
Root Cause Analysis
The UI could be driven to take ownership of a PasswordFormManager whose FormFetcher was still in State::WAITING, i.e. its credential fetch had not completed. The removed check in PasswordManager (GetFormFetcher()->GetState() == FormFetcher::State::WAITING) was the only thing standing between a pending-fetch manager and the fallback/UI path, and the invariant that ManagePasswordsState only ever receives a fetch-completed manager was never enforced. When the manual fallback UI was shown for such a manager and the pending fetch then resolved, the OnFetchCompleted callback could run and synchronously tear down or replace the PasswordFormManager while it was still referenced by the UI, producing a use-after-free (as the commit subject states, “not destroyed synchronously during OnFetchCompleted”).
The fix introduces an explicit IsFetchCompleted() predicate, suppresses the manual fallback while the fetch is pending, and adds CHECK(form_manager->IsFetchCompleted()) at every ManagePasswordsState entry point so a not-yet-fetched manager can never be installed into UI state.
PasswordFormManager whose asynchronous store fetch was still in flight, so the later OnFetchCompleted could free an object the UI still held; the fix enforces the “fetch must be complete before UI ownership” invariant with IsFetchCompleted() guards and a CHECK.Attack Path
- Observe a form before fetch resolves
The page registers a password form, which starts the asynchronous
FormFetcherquery; the manager’s state isFormFetcher::State::WAITING. - Trigger input before the fetch completes
The user (or scripted input) types a value into the password field so
OnInformAboutUserInputruns while the fetch is still pending, as modeled by theNoManualFallbackWhenFetchIsPendingregression test. - Surface the manual fallback UI
Pre-patch, the manual-fallback path handed the pending-fetch
PasswordFormManagerto the UI, which took ownership of it. - Resolve the fetch
When the store query completes,
OnFetchCompletedruns and can synchronously destroy/replace thePasswordFormManagerwhile the UI still references it, creating a dangling pointer that is subsequently dereferenced (use-after-free) (inferred as to the exact dereference site; the diff shows the ordering hazard but not the sink).
Impact Assessment
(inferred) and not demonstrated by the diff.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
Audit Directions
- Async ownership transferAudit every site that
std::moves aPasswordFormManagerForUIinto UI state and confirm the fetch is complete first; a manager still inState::WAITINGcan be freed by its own completion callback. - Completion-callback reentrancyLook for callbacks like
OnFetchCompletedthat can synchronously destroy or replace the object currently held by an outer caller, especially where the object is passed by raw/borrowed reference across the UI boundary. - Invariant enforced only by one caller-side checkWhere a single
GetState() == WAITINGstyle guard was the only protection, prefer a named predicate plusCHECKs at the ownership sinks so the invariant holds regardless of which code path delivers the object.
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.