diff --git a/remoting/protocol/session_authz_authenticator_unittest.cc b/remoting/protocol/session_authz_authenticator_unittest.cc index a3ae3fb552615..9da435b4a3207 100644 --- a/remoting/protocol/session_authz_authenticator_unittest.cc +++ b/remoting/protocol/session_authz_authenticator_unittest.cc @@ -453,6 +453,85 @@ TEST_F(SessionAuthzAuthenticatorTest, Authenticator::RejectionReason::UNEXPECTED_ERROR); } +// ============================================================================= +// POC: heap-use-after-free in StartReauthorizerIfNecessary(). +// +// In production, the |resume_callback| passed into +// SessionAuthzAuthenticator::ProcessMessage() is +// NegotiatingAuthenticatorBase::UpdateState +// -> JingleSession::ProcessAuthenticationStep. +// +// Spake2Authenticator::ProcessMessage() runs that callback *synchronously* as +// its last statement. When the SPAKE2 step ends in REJECTED (e.g. the attacker +// sends an empty ), ProcessAuthenticationStep -> Close() -> +// SetState(FAILED) -> WebrtcConnectionToClient::OnSessionStateChange -> +// ClientSession::OnConnectionClosed -> ChromotingHost::OnSessionClosed -> +// clients_.erase(it), which synchronously destroys ClientSession -> +// ConnectionToClient -> Session -> Authenticator -> SessionAuthzAuthenticator. +// +// Control then returns to session_authz_authenticator.cc:249 (or :124), which +// calls StartReauthorizerIfNecessary() on freed |this|. +// +// This test models that production destruction chain by having the +// resume_callback delete the authenticator (exactly what clients_.erase() +// does), then ASAN catches the heap-use-after-free. +// ============================================================================= +TEST_F(SessionAuthzAuthenticatorTest, + UAF_ResumeCallbackDestroysAuthenticatorOnRejection) { + EXPECT_CALL(*mock_service_client_, GenerateHostToken(_)) + .WillOnce(RespondGenerateHostToken()); + // VerifySessionToken succeeds (precondition: attacker is authorized by the + // SessionAuthz backend). The mock runs the callback synchronously; in + // production this is async, but the UAF site is inside + // OnVerifiedSessionToken() so synchronicity of the HTTP layer is irrelevant. + EXPECT_CALL(*mock_service_client_, VerifySessionToken(kFakeSessionToken, _)) + .WillOnce(RespondVerifySessionToken()); + + // Drive the authenticator to WAITING_FOR_SESSION_TOKEN. + base::RunLoop run_loop; + host_authenticator_->Start(run_loop.QuitClosure()); + run_loop.Run(); + ASSERT_EQ(host_->state(), Authenticator::MESSAGE_READY); + std::ignore = host_->GetNextMessage(); + ASSERT_EQ(host_->state(), Authenticator::WAITING_MESSAGE); + + // Attacker-controlled session-info: valid session_token, EMPTY spake_message. + // Empty spake_message drives Spake2Authenticator straight to REJECTED + // (spake2_authenticator.cc:155-159) without needing the shared secret. + JingleAuthentication attacker_msg; + attacker_msg.session_authz_session_token = std::string(kFakeSessionToken); + // attacker_msg.spake_message intentionally left empty. + + // Snapshot the call target; clear the test fixture's raw_ptr observers so + // dangling-ptr checks don't fire and obscure the real bug. + Authenticator* host_raw = host_.get(); + mock_service_client_ = nullptr; + host_authenticator_ = nullptr; + client_authenticator_ = nullptr; + + // resume_callback that models production: + // UpdateState -> ProcessAuthenticationStep -> Close() -> ... -> + // ChromotingHost::OnSessionClosed -> clients_.erase(it). + auto resume_callback = base::BindOnce( + [](std::unique_ptr* owner) { + // UpdateState: read the underlying state. + ASSERT_EQ((*owner)->state(), Authenticator::REJECTED); + // ProcessAuthenticationStep: state == REJECTED -> Close() -> + // ... -> clients_.erase(): synchronously free the authenticator + // while SessionAuthzAuthenticator::OnVerifiedSessionToken's frame + // is still on the stack. + owner->reset(); + }, + &host_); + + // ProcessMessage -> VerifySessionToken -> OnVerifiedSessionToken: + // underlying_ = Spake2Authenticator(WAITING_MESSAGE) + // underlying_->ProcessMessage(attacker_msg, resume_callback) + // -> REJECTED -> resume_callback.Run() -> host_.reset() [frees |this|] + // StartReauthorizerIfNecessary() <-- heap-use-after-free on |this| + host_raw->ProcessMessage(attacker_msg, std::move(resume_callback)); +} + TEST_F(SessionAuthzAuthenticatorTest, ReauthorizationFailed_Rejected) { EXPECT_CALL(*mock_service_client_, GenerateHostToken(_)) .WillOnce(RespondGenerateHostToken());