diff --git a/components/password_manager/core/browser/password_autofill_manager_unittest.cc b/components/password_manager/core/browser/password_autofill_manager_unittest.cc index 270d1342ea604..2ce71e8784323 100644 --- a/components/password_manager/core/browser/password_autofill_manager_unittest.cc +++ b/components/password_manager/core/browser/password_autofill_manager_unittest.cc @@ -1208,6 +1208,87 @@ TEST_F(PasswordAutofillManagerTest, FillsSuggestionIfAuthSuccessful) { SuggestionPosition{.row = 1}); } +// A DeviceAuthenticator that simulates DeviceAuthenticatorMac's +// non-biometric path: AuthenticateWithMessage() synchronously runs a nested +// event loop (modeled here as |on_nested_loop_|), during which the page that +// owns the PasswordAutofillManager can be destroyed. After the nested loop +// returns, the authenticator's own weak_this guard fires and the method +// returns without touching members or running |callback| -- exactly what +// device_authenticator_mac.mm does at lines 91-98 when weak_this is null. +class NestedLoopDestroysOwnerAuthenticator + : public device_reauth::DeviceAuthenticator { + public: + explicit NestedLoopDestroysOwnerAuthenticator(base::OnceClosure on_nested_loop) + : on_nested_loop_(std::move(on_nested_loop)) {} + ~NestedLoopDestroysOwnerAuthenticator() override = default; + + bool CanAuthenticateWithBiometrics() override { return false; } + bool CanAuthenticateWithBiometricOrScreenLock() override { return true; } + void Cancel() override {} + void AuthenticateWithMessage(const std::u16string& message, + AuthenticateCallback callback) override { + // Snapshot the closure onto the stack: running it destroys |*this| + // (via ~PasswordAutofillManager -> CancelBiometricReauthIfOngoing -> + // authenticator_.reset()). + base::OnceClosure nested = std::move(on_nested_loop_); + // Drop |callback| like the production weak_this==null path does. + callback.Reset(); + // Simulate the synchronous nested CFRunLoop that lets the opener + // destroy the popup's PasswordAutofillManager. + std::move(nested).Run(); + // |*this| is now freed. Return without touching any members, just like + // device_authenticator_mac.mm:95-98 when weak_this is null. + } + + private: + base::OnceClosure on_nested_loop_; +}; + +// Regression test: on macOS, AuthenticateWithMessage() can spin a nested +// CFRunLoop (AuthorizationCopyRights) during which the RenderFrameHost owning +// |this| PasswordAutofillManager is freed. The fix for the +// DeviceAuthenticatorMac frame guards that frame, but DidAcceptSuggestion() +// still dereferences member fields of |this| after the synchronous call +// returns. Under ASAN this test produces a heap-use-after-free at the +// password_client_->GetWebAuthnCredentialsDelegateForDriver() call. +TEST_F(PasswordAutofillManagerTest, + DidAcceptSuggestion_ManagerFreedDuringSyncReauth_UAF) { + TestPasswordManagerClient client; + NiceMock autofill_client; + InitializePasswordAutofillManager(&client, &autofill_client); + + password_autofill_manager_->ShowSuggestions(kTriggeringField); + + // Force the reauth-before-filling path (production: macOS with the + // kBiometricAuthenticationBeforeFilling pref set and biometrics currently + // unavailable). + EXPECT_CALL(client, IsReauthBeforeFillingRequired).WillOnce(Return(true)); + EXPECT_CALL(client, GetDeviceAuthenticator) + .WillOnce(Return(testing::ByMove( + std::make_unique( + base::BindOnce( + [](std::unique_ptr* mgr) { + // Simulates ContentPasswordManagerDriverFactory:: + // RenderFrameDeleted erasing the map node that owns the + // by-value PasswordAutofillManager while the macOS + // password dialog's nested run loop is spinning. + mgr->reset(); + }, + &password_autofill_manager_))))); + + // Hold a bare pointer the way AutofillPopupControllerImpl does after + // dereferencing its WeakPtr. + PasswordAutofillManager* delegate = password_autofill_manager_.get(); + delegate->DidAcceptSuggestion( + autofill::test::CreateAutofillSuggestion( + autofill::SuggestionType::kPasswordEntry, test_username_), + SuggestionPosition{.row = 1}); + // If the bug is fixed, |delegate| was freed mid-call and DidAcceptSuggestion + // returned early without touching members. If not, ASAN reports a + // heap-use-after-free above. + EXPECT_EQ(password_autofill_manager_, nullptr); +} + TEST_F(PasswordAutofillManagerTest, DoesntFillSuggestionIfAuthFailed) { TestPasswordManagerClient client; NiceMock autofill_client;