diff --git a/content/browser/webauth/authenticator_impl_unittest.cc b/content/browser/webauth/authenticator_impl_unittest.cc index 8edc66355c65f..07fe9b422a842 100644 --- a/content/browser/webauth/authenticator_impl_unittest.cc +++ b/content/browser/webauth/authenticator_impl_unittest.cc @@ -10709,4 +10709,117 @@ TEST_F(AuthenticatorImplTest, CrossDeviceFallbackUrl_Processed) { EXPECT_EQ(result.status, AuthenticatorStatus::CROSS_DEVICE_FALLBACK); } +// ----------------------------------------------------------------------------- +// Regression coverage for AuthenticatorCommonImpl::Cleanup() use-after-free. +// +// Cleanup() does: +// req_state_.reset(); // synchronously destroys the request delegate +// next_request_key_++; // unconditional write to `this` +// CHECK(next_request_key_); +// +// In production the request delegate is ChromeAuthenticatorRequestDelegate. +// Its destructor (via dialog_model_->OnRequestComplete -> SetStep(kClosed) -> +// view_controller_.reset()) synchronously destroys a CLIENT_OWNS_WIDGET +// constrained tab-modal views::Widget. As documented for the structurally +// identical Digital Credentials code paths +// (content/browser/digital_credentials/digital_identity_request_impl.cc and +// chrome/browser/digital_credentials/digital_identity_provider_desktop.cc), +// that widget teardown can — via activation observers — synchronously destroy +// the hosting WebContents (e.g. when the relying-party page lives in a +// transient container such as an extension popup that closes on activation +// change). That in turn destroys the frame-bound DocumentService +// AuthenticatorImpl and the AuthenticatorCommonImpl it owns, freeing `this` +// while Cleanup() is still on the stack. +// +// This test models that destruction directly in the same way the +// DigitalIdentityRequestImpl regression test does: the embedder-supplied +// request delegate's destructor synchronously deletes the harness WebContents. +// Under ASan this surfaces as a heap-use-after-free WRITE on +// `next_request_key_++` inside AuthenticatorCommonImpl::Cleanup(). + +class WebAuthnHostDestroyingRequestDelegate + : public DefaultAuthenticatorRequestClientDelegate { + public: + explicit WebAuthnHostDestroyingRequestDelegate(base::OnceClosure on_destroy) + : on_destroy_(std::move(on_destroy)) {} + ~WebAuthnHostDestroyingRequestDelegate() override { + if (on_destroy_) { + std::move(on_destroy_).Run(); + } + } + + private: + base::OnceClosure on_destroy_; +}; + +class WebAuthnHostDestroyingBrowserClient : public ContentBrowserClient { + public: + WebAuthenticationDelegate* GetWebAuthenticationDelegate() override { + return &web_authentication_delegate_; + } + std::unique_ptr + GetWebAuthenticationRequestDelegate( + RenderFrameHost* render_frame_host) override { + return std::make_unique( + std::move(on_destroy_)); + } + + void set_on_destroy(base::OnceClosure cb) { on_destroy_ = std::move(cb); } + + private: + TestWebAuthenticationDelegate web_authentication_delegate_; + base::OnceClosure on_destroy_; +}; + +class AuthenticatorImplCleanupUAFTest : public AuthenticatorImplTest { + public: + void SetUp() override { + AuthenticatorImplTest::SetUp(); + old_client_ = SetBrowserClientForTesting(&test_client_); + } + void TearDown() override { + SetBrowserClientForTesting(old_client_); + AuthenticatorImplTest::TearDown(); + } + + protected: + WebAuthnHostDestroyingBrowserClient test_client_; + raw_ptr old_client_ = nullptr; +}; + +TEST_F(AuthenticatorImplCleanupUAFTest, + CleanupAfterRequestDelegateDestroysWebContents) { + NavigateAndCommit(GURL(kTestOrigin1)); + + // Arrange for the embedder request delegate's destructor to synchronously + // tear down the hosting WebContents — modelling the production scenario + // where ~ChromeAuthenticatorRequestDelegate -> ~views::Widget -> + // activation-observer destroys a transient host (e.g. extension popup). + bool delegate_destroyed = false; + test_client_.set_on_destroy(base::BindLambdaForTesting([&] { + delegate_destroyed = true; + DeleteContents(); + })); + + // Drive a full MakeCredential round-trip on the virtual authenticator. + // Completion runs CompleteMakeCredentialRequest() -> Cleanup(): + // req_state_.reset() // ~RequestState + // ~WebContentsDestroyingRequestDelegate + // DeleteContents() + // ~WebContentsImpl -> ~RenderFrameHostImpl -> + // DocumentService::~AuthenticatorImpl -> + // ~AuthenticatorCommonImpl // frees `this` + // next_request_key_++; // <-- UAF WRITE on freed `this` + // CHECK(next_request_key_); // <-- UAF READ + mojo::Remote authenticator = + ConnectToAuthenticator(); + TestMakeCredentialFuture future; + authenticator->MakeCredential(GetTestPublicKeyCredentialCreationOptions(), + future.GetCallback()); + // The Mojo response is sent before Cleanup(), so the future resolves even + // though the receiver is torn down inside Cleanup(). + EXPECT_TRUE(future.Wait()); + EXPECT_TRUE(delegate_destroyed); +} + } // namespace content