diff --git a/chrome/browser/webauthn/chrome_webauthn_browsertest.cc b/chrome/browser/webauthn/chrome_webauthn_browsertest.cc index 5e912ff505fd9..1308a21e628b6 100644 --- a/chrome/browser/webauthn/chrome_webauthn_browsertest.cc +++ b/chrome/browser/webauthn/chrome_webauthn_browsertest.cc @@ -1864,4 +1864,170 @@ IN_PROC_BROWSER_TEST_F( testing::HasSubstr("error SecurityError")); } +// ===================================================================== +// Browser-process UAF: AuthenticatorRequestDialogController:: +// CancelAuthenticatorRequest reads `this->model_` after +// SetCurrentStep(kClosed) synchronously frees the controller. +// +// This test simulates the activation-observer chain documented at +// content/browser/webauth/authenticator_common_impl.cc (Cleanup()): closing +// the constrained WebAuthn dialog Widget can, via activation observers, +// synchronously destroy the hosting WebContents, which destroys +// AuthenticatorImpl -> AuthenticatorCommonImpl -> RequestState -> +// ChromeAuthenticatorRequestDelegate -> dialog_controller_ (the controller +// whose CancelAuthenticatorRequest() is mid-execution on the stack). +// ===================================================================== +class WebAuthnCancelUAFTest : public WebAuthnBrowserTest { + public: + // Simulates the activation-observer side-effect: when the model transitions + // to kClosed (which happens inside SetCurrentStep(kClosed) right after the + // dialog Widget is synchronously destroyed via view_controller_.reset()), + // synchronously destroy the hosting WebContents. + class TabCloser : public AuthenticatorRequestDialogModel::Observer { + public: + TabCloser(Browser* browser, + content::WebContents* web_contents, + AuthenticatorRequestDialogModel* model) + : browser_(browser), web_contents_(web_contents), model_(model) { + model_->observers.AddObserver(this); + } + ~TabCloser() override { + if (model_) { + model_->observers.RemoveObserver(this); + } + } + void OnStepTransition() override { + if (fired_ || !model_ || + model_->step() != AuthenticatorRequestDialogModel::Step::kClosed) { + return; + } + fired_ = true; + content::WebContents* wc = web_contents_; + web_contents_ = nullptr; + const int idx = + browser_->tab_strip_model()->GetIndexOfWebContents(wc); + ASSERT_NE(idx, TabStripModel::kNoTab); + // Synchronously destroys the WebContents, which destroys the + // AuthenticatorImpl DocumentService and the entire request-delegate + // chain that owns the AuthenticatorRequestDialogController. + browser_->tab_strip_model()->DetachAndDeleteWebContentsAt(idx); + } + void OnModelDestroyed(AuthenticatorRequestDialogModel*) override { + model_ = nullptr; + } + + private: + bool fired_ = false; + raw_ptr browser_; + raw_ptr web_contents_; + raw_ptr model_; + }; + + class DelegateObserver + : public ChromeAuthenticatorRequestDelegate::TestObserver { + public: + virtual ~DelegateObserver() = default; + void WaitForUI() { run_loop_.Run(); } + ChromeAuthenticatorRequestDelegate* delegate() { return delegate_; } + + void UIShown(ChromeAuthenticatorRequestDelegate* delegate) override { + delegate_ = delegate; + run_loop_.QuitWhenIdle(); + } + void OnDestroy(ChromeAuthenticatorRequestDelegate* delegate) override { + if (delegate_ == delegate) { + delegate_ = nullptr; + } + } + + private: + base::RunLoop run_loop_; + raw_ptr + delegate_ = nullptr; + }; + + void SetUpOnMainThread() override { + WebAuthnBrowserTest::SetUpOnMainThread(); + observer_ = std::make_unique(); + auto factory = std::make_unique(); + auth_env_ = + std::make_unique( + std::move(factory)); + ChromeAuthenticatorRequestDelegate::SetGlobalObserverForTesting( + observer_.get()); + } + + void PostRunTestOnMainThread() override { + ChromeAuthenticatorRequestDelegate::SetGlobalObserverForTesting(nullptr); + auth_env_.reset(); + WebAuthnBrowserTest::PostRunTestOnMainThread(); + } + + protected: + std::unique_ptr observer_; + std::unique_ptr auth_env_; +}; + +IN_PROC_BROWSER_TEST_F(WebAuthnCancelUAFTest, + CancelAfterErrorFreesControllerThenReadsModel) { + // Open a second tab so destroying the WebAuthn tab does not close the + // browser. + ASSERT_TRUE(AddTabAtIndex( + 0, https_server_.GetURL("www.example.com", "/title1.html"), + ui::PAGE_TRANSITION_TYPED)); + ASSERT_EQ(browser()->tab_strip_model()->count(), 2); + + // Navigate the active (new) tab and start a WebAuthn get() request. + ASSERT_TRUE(ui_test_utils::NavigateToURL( + browser(), https_server_.GetURL("www.example.com", "/title1.html"))); + content::WebContents* const web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + + content::ExecuteScriptAsync(web_contents, R"((() => { + let cred_id = new Uint8Array([1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16]); + navigator.credentials.get({ publicKey: { + challenge: cred_id, + timeout: 60000, + userVerification: 'discouraged', + allowCredentials: [{type: 'public-key', id: cred_id}], + }}).catch(e => {}); + })())"); + observer_->WaitForUI(); + + ChromeAuthenticatorRequestDelegate* const delegate = observer_->delegate(); + ASSERT_TRUE(delegate); + + // Hold the model alive across delegate destruction so we isolate the + // controller-layer UAF (the model is RefCounted; the delegate holds the only + // other ref). + scoped_refptr model = + delegate->dialog_model(); + + // Drive the request into a DIALOG-type error state where + // is_request_complete() is true (e.g. the user reaches the "timed out" + // sheet). If the virtual device already drove the request to a complete + // state (e.g. kKeyNotRegistered) this is a no-op. + delegate->dialog_controller()->OnRequestTimeout(); + ASSERT_TRUE(delegate->dialog_controller()->is_request_complete()); + + // Install the simulated activation-observer that destroys the WebContents + // when the dialog closes. + TabCloser closer(browser(), web_contents, model.get()); + + // Simulate the user clicking Cancel on the error sheet. + // This invokes the model's CancelAuthenticatorRequest broadcast, which + // calls into AuthenticatorRequestDialogController::CancelAuthenticatorRequest + // (controller.cc:611). is_request_complete() is true, so it calls + // SetCurrentStep(kClosed) -> SetStep(kClosed) -> view_controller_.reset() + // (synchronously destroys the Widget) -> OnStepTransition observers fire -> + // TabCloser destroys the WebContents -> ~AuthenticatorImpl -> + // ~AuthenticatorCommonImpl -> ~RequestState -> + // ~ChromeAuthenticatorRequestDelegate -> dialog_controller_.reset() frees + // the controller. Control then unwinds back to controller.cc:631 which reads + // `this->model_` from the freed controller allocation. + // + // Under ASan: heap-use-after-free on AuthenticatorRequestDialogController. + model->CancelAuthenticatorRequest(); +} + } // namespace