diff --git a/components/autofill/core/browser/ui/payments/autofill_progress_dialog_controller_impl_unittest.cc b/components/autofill/core/browser/ui/payments/autofill_progress_dialog_controller_impl_unittest.cc index 82d408a4e9eb3c..c855ae4a8a6a13 100644 --- a/components/autofill/core/browser/ui/payments/autofill_progress_dialog_controller_impl_unittest.cc +++ b/components/autofill/core/browser/ui/payments/autofill_progress_dialog_controller_impl_unittest.cc @@ -6,13 +6,53 @@ #include +#include "base/functional/bind.h" +#include "base/functional/callback_helpers.h" +#include "base/memory/raw_ptr.h" #include "base/test/metrics/histogram_tester.h" #include "components/autofill/core/browser/ui/payments/autofill_progress_dialog_view.h" +#include "components/autofill/core/browser/ui/payments/autofill_progress_ui_type.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" namespace autofill { +// A test view that mimics the production desktop synchronous-close path: +// AutofillProgressDialogViews::Dismiss() -> CloseWidget() -> +// Widget::CloseWithReason() -> override_close_ (registered via +// MakeCloseSynchronous in autofill_progress_dialog_views.cc) -> +// AutofillProgressDialogViewImpl::CloseWidget() -> +// controller()->OnDismissed(is_canceled_by_user). +class SyncDismissTestAutofillProgressDialogView + : public AutofillProgressDialogView { + public: + explicit SyncDismissTestAutofillProgressDialogView( + AutofillProgressDialogController* controller) + : controller_(controller) {} + ~SyncDismissTestAutofillProgressDialogView() override = default; + + void Dismiss(bool show_confirmation_before_closing, + bool is_canceled_by_user) override { + // Capture the controller pointer to a local: `this` is destroyed inside + // OnDismissed() at line 71 (autofill_progress_dialog_view_.reset()), which + // matches production behaviour (the AutofillProgressDialogViewImpl + // unique_ptr is reset while the widget-close re-entrancy is on the stack). + AutofillProgressDialogController* controller = controller_.get(); + controller_ = nullptr; + controller->OnDismissed(is_canceled_by_user); + // `this` is freed; do not touch members. + } + void InvalidateControllerForCallbacks() override { controller_ = nullptr; } + base::WeakPtr GetWeakPtr() override { + return weak_ptr_factory_.GetWeakPtr(); + } + + private: + raw_ptr controller_; + base::WeakPtrFactory + weak_ptr_factory_{this}; +}; + class TestAutofillProgressDialogView : public AutofillProgressDialogView { public: TestAutofillProgressDialogView() = default; @@ -51,4 +91,61 @@ class AutofillProgressDialogControllerImplTest : public testing::Test { std::unique_ptr controller_; }; +// Regression test modelling the macOS production scenario: +// +// 1. CreditCardAccessManager::OnNonInteractiveAuthenticationSuccess() calls +// CloseAutofillProgressDialog(false, BindOnce(&StartDeviceAuthentication +// ForFilling, ...)) which calls DismissDialog(). +// 2. DismissDialog() -> view->Dismiss() synchronously re-enters +// OnDismissed(false) via Widget::override_close_ (MakeCloseSynchronous). +// 3. OnDismissed() runs no_interactive_authentication_callback_ -> +// StartDeviceAuthenticationForFilling -> ... -> +// AuthorizationCopyRights(), which spins a nested CFRunLoop on the +// browser UI thread (documented at device_authenticator_mac.mm:88-90). +// 4. The opener executes popup.close(); the nested run loop processes the +// close IPC and destroys the popup WebContents -> ChromeAutofillClient +// -> ChromePaymentsAutofillClient -> this controller is freed. +// 5. AuthorizationCopyRights returns; OnDismissed() lines 80-82 read +// `this->autofill_progress_dialog_type_` and call +// `this->cancel_callback_.Reset()` on freed memory. +// +// This test models step 4 by passing a no_interactive_authentication_callback +// that directly destroys the controller's owning unique_ptr (the same effect +// the nested run loop achieves on macOS). Under ASAN this fires +// heap-use-after-free in OnDismissed(). +TEST(AutofillProgressDialogControllerImplUAFTest, + OnDismissedAfterCallbackDestroysController) { + std::unique_ptr controller = + std::make_unique( + AutofillProgressUiType::kServerCardUnmaskProgressUi, + /*cancel_callback=*/base::DoNothing()); + AutofillProgressDialogControllerImpl* const controller_raw = controller.get(); + + // Step 1/2 setup: show the dialog with a view that synchronously re-enters + // OnDismissed() when Dismiss() is called (production behaviour via + // MakeCloseSynchronous). + controller_raw->ShowDialog(base::BindOnce( + [](AutofillProgressDialogController* c) + -> std::unique_ptr { + return std::make_unique(c); + }, + base::Unretained(controller_raw))); + + // Step 3/4: DismissDialog stores the callback and calls view->Dismiss() + // -> OnDismissed(false) -> runs the callback -> controller is freed + // (modelling WebContents destruction inside the macOS nested run loop). + controller_raw->DismissDialog( + /*show_confirmation_before_closing=*/false, + /*no_interactive_authentication_callback=*/base::BindOnce( + [](std::unique_ptr* owner) { + owner->reset(); + }, + base::Unretained(&controller))); + + // Step 5: OnDismissed() lines 80-82 have already executed with freed `this` + // by the time control returns here. Under ASAN this is a + // heap-use-after-free. In a non-ASAN build the test "passes" silently. + SUCCEED(); +} + } // namespace autofill