CVE-2026-11116
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifremoting/protocol/negotiating_authenticator_base.cc |
modified | |
TestNegotiatingHostAuthenticatorremoting/protocol/negotiating_authenticator_unittest.cc |
modified | |
ProxyAuthenticatorremoting/protocol/negotiating_authenticator_unittest.cc |
modified | |
NegotiatingAuthenticatorTestremoting/protocol/negotiating_authenticator_unittest.cc |
modified |
Files Changed
remoting/protocol/authenticator.ccremoting/protocol/authenticator.hremoting/protocol/negotiating_authenticator_base.ccremoting/protocol/negotiating_authenticator_base.hremoting/protocol/negotiating_authenticator_unittest.cc
Patch
From 174583c9fcf83d5f5b5d1abc841366f6c8986556 Mon Sep 17 00:00:00 2001 From: Joe Downing <[email protected]> Date: Sat, 11 Apr 2026 08:34:25 -0700 Subject: [PATCH] Fix more potential UaF instances More potential issues found while spelunking through the impl. Bug: 501376612 Change-Id: I63e385dadd23c8e8b86c3f068c73bdd22c88d768 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7749841 Reviewed-by: Yuwei Huang <[email protected]> Commit-Queue: Joe Downing <[email protected]> Cr-Commit-Position: refs/heads/main@{#1613295} --- diff --git a/remoting/protocol/authenticator.cc b/remoting/protocol/authenticator.cc index 3495f435..018db9f 100644 --- a/remoting/protocol/authenticator.cc +++ b/remoting/protocol/authenticator.cc @@ -41,8 +41,9 @@ void Authenticator::ChainStateChangeAfterAcceptedWithUnderlying( Authenticator& underlying) { - underlying.set_state_change_after_accepted_callback(base::BindRepeating( - &Authenticator::NotifyStateChangeAfterAccepted, base::Unretained(this))); + underlying.set_state_change_after_accepted_callback( + base::BindRepeating(&Authenticator::NotifyStateChangeAfterAccepted, + weak_factory_.GetWeakPtr())); } } // namespace remoting::protocol diff --git a/remoting/protocol/authenticator.h b/remoting/protocol/authenticator.h index 340fdc9..32c3022 100644 --- a/remoting/protocol/authenticator.h +++ b/remoting/protocol/authenticator.h @@ -11,6 +11,7 @@ #include "base/functional/callback.h" #include "base/location.h" +#include "base/memory/weak_ptr.h" #include "remoting/base/session_policies.h" #include "remoting/protocol/credentials_type.h" #include "remoting/signaling/jingle_data_structures.h" @@ -224,6 +225,8 @@ private: base::RepeatingClosure on_state_change_after_accepted_; + + base::WeakPtrFactory<Authenticator> weak_factory_{this}; }; // Factory for Authenticator instances. diff --git a/remoting/protocol/negotiating_authenticator_base.cc b/remoting/protocol/negotiating_authenticator_base.cc index 897723fd..52fa6ec 100644 --- a/remoting/protocol/negotiating_authenticator_base.cc +++ b/remoting/protocol/negotiating_authenticator_base.cc @@ -71,11 +71,10 @@ if (current_authenticator_->state() == WAITING_MESSAGE) { // If the message was not discarded and the authenticator is waiting for it, // give it to the underlying authenticator to process. - // |current_authenticator_| is owned, so Unretained() is safe here. current_authenticator_->ProcessMessage( message, base::BindOnce(&NegotiatingAuthenticatorBase::UpdateState, - base::Unretained(this), std::move(resume_callback))); + weak_factory_.GetWeakPtr(), std::move(resume_callback))); } else { // Otherwise, just discard the message. UpdateState(std::move(resume_callback)); @@ -108,9 +107,15 @@ DCHECK(current_method_ != AuthenticationMethod::INVALID); JingleAuthentication result; + auto self = weak_factory_.GetWeakPtr(); if (current_authenticator_->state() == MESSAGE_READY) { result = current_authenticator_->GetNextMessage(); } + + if (!self) { + return result; + } + state_ = current_authenticator_->state(); // |state_| may be MESSAGE_READY if the underlying authenticator has // multiple messages to send. diff --git a/remoting/protocol/negotiating_authenticator_base.h b/remoting/protocol/negotiating_authenticator_base.h index 7b4cfa43..bb7c9233 100644 --- a/remoting/protocol/negotiating_authenticator_base.h +++ b/remoting/protocol/negotiating_authenticator_base.h @@ -9,6 +9,7 @@ #include <string> #include <vector> +#include "base/memory/weak_ptr.h" #include "remoting/protocol/authenticator.h" #include "remoting/protocol/host_authentication_config.h" @@ -104,6 +105,8 @@ State state_; RejectionReason rejection_reason_ = RejectionReason::INVALID_CREDENTIALS; RejectionDetails rejection_details_; + + base::WeakPtrFactory<NegotiatingAuthenticatorBase> weak_factory_{this}; }; } // namespace remoting::protocol diff --git a/remoting/protocol/negotiating_authenticator_unittest.cc b/remoting/protocol/negotiating_authenticator_unittest.cc index 441b6601..b4e9a99 100644 --- a/remoting/protocol/negotiating_authenticator_unittest.cc +++ b/remoting/protocol/negotiating_authenticator_unittest.cc @@ -4,6 +4,7 @@ #include <algorithm> #include <memory> +#include <tuple> #include "base/functional/bind.h" #include "base/memory/raw_ptr.h" @@ -52,6 +53,66 @@ const char kTestPin[] = "123456"; const char kTestPinBad[] = "654321"; +class TestNegotiatingHostAuthenticator : public NegotiatingHostAuthenticator { + public: + using Authenticator::ChainStateChangeAfterAcceptedWithUnderlying; + using NegotiatingAuthenticatorBase::current_authenticator_; + using NegotiatingAuthenticatorBase::current_method_; + using NegotiatingAuthenticatorBase::NotifyStateChangeAfterAccepted; + using NegotiatingAuthenticatorBase::state_; + using NegotiatingHostAuthenticator::NegotiatingHostAuthenticator; + + base::WeakPtr<NegotiatingAuthenticatorBase> GetWeakPtr() { + return weak_factory_.GetWeakPtr(); + } + + void NotifyStateChangeAfterAcceptedForTesting() { + NotifyStateChangeAfterAccepted(); + } +}; + +class ProxyAuthenticator : public Authenticator { + public: + explicit ProxyAuthenticator(Authenticator* authenticator) + : authenticator_(authenticator) {} + ~ProxyAuthenticator() override = default; + + CredentialsType credentials_type() const override { + return authenticator_->credentials_type(); + } + const Authenticator& implementing_authenticator() const override { + return authenticator_->implementing_authenticator(); + } + State state() const override { return authenticator_->state(); } + bool started() const override { return authenticator_->started(); } + RejectionReason rejection_reason() const override { + return authenticator_->rejection_reason(); + } + RejectionDetails rejection_details() const override { + return authenticator_->rejection_details(); + } + void ProcessMessage(const JingleAuthentication& message, + base::OnceClosure resume_callback) override { + authenticator_->ProcessMessage(message, std::move(resume_callback)); + } + JingleAuthentication GetNextMessage() override { + return authenticator_->GetNextMessage(); + } + const std::string& GetAuthKey() const override { + return authenticator_->GetAuthKey(); + } + const SessionPolicies* GetSessionPolicies() const override { + return authenticator_->GetSessionPolicies(); + } + std::unique_ptr<ChannelAuthenticator> CreateChannelAuthenticator() + const override { + return authenticator_->CreateChannelAuthenticator(); + } + + private: + raw_ptr<Authenticator> authenticator_; +}; + } // namespace class NegotiatingAuthenticatorTest : public AuthenticatorTestBase { @@ -176,6 +237,19 @@ raw_ptr<NegotiatingHostAuthenticator> host_as_negotiating_authenticator_; raw_ptr<NegotiatingClientAuthenticator> client_as_negotiating_authenticator_; + protected: + void SetHostState(Authenticator::State state) { + host_as_negotiating_authenticator_->state_ = state; + } + + void SetHostCurrentMethod(AuthenticationMethod method) { + host_as_negotiating_authenticator_->current_method_ = method; + } + + void NotifyHostStateChangeAfterAccepted() {
Regression Test / PoC
diff --git a/remoting/protocol/negotiating_authenticator_unittest.cc b/remoting/protocol/negotiating_authenticator_unittest.cc
index 441b6601..b4e9a99 100644
--- a/remoting/protocol/negotiating_authenticator_unittest.cc
+++ b/remoting/protocol/negotiating_authenticator_unittest.cc
@@ -4,6 +4,7 @@
#include <algorithm>
#include <memory>
+#include <tuple>
#include "base/functional/bind.h"
#include "base/memory/raw_ptr.h"
@@ -52,6 +53,66 @@
const char kTestPin[] = "123456";
const char kTestPinBad[] = "654321";
+class TestNegotiatingHostAuthenticator : public NegotiatingHostAuthenticator {
+ public:
+ using Authenticator::ChainStateChangeAfterAcceptedWithUnderlying;
+ using NegotiatingAuthenticatorBase::current_authenticator_;
+ using NegotiatingAuthenticatorBase::current_method_;
+ using NegotiatingAuthenticatorBase::NotifyStateChangeAfterAccepted;
+ using NegotiatingAuthenticatorBase::state_;
+ using NegotiatingHostAuthenticator::NegotiatingHostAuthenticator;
+
+ base::WeakPtr<NegotiatingAuthenticatorBase> GetWeakPtr() {
+ return weak_factory_.GetWeakPtr();
+ }
+
+ void NotifyStateChangeAfterAcceptedForTesting() {
+ NotifyStateChangeAfterAccepted();
+ }
+};
+
+class ProxyAuthenticator : public Authenticator {
+ public:
+ explicit ProxyAuthenticator(Authenticator* authenticator)
+ : authenticator_(authenticator) {}
+ ~ProxyAuthenticator() override = default;
+
+ CredentialsType credentials_type() const override {
+ return authenticator_->credentials_type();
+ }
+ const Authenticator& implementing_authenticator() const override {
+ return authenticator_->implementing_authenticator();
+ }
+ State state() const override { return authenticator_->state(); }
+ bool started() const override { return authenticator_->started(); }
+ RejectionReason rejection_reason() const override {
+ return authenticator_->rejection_reason();
+ }
+ RejectionDetails rejection_details() const override {
+ return authenticator_->rejection_details();
+ }
+ void ProcessMessage(const JingleAuthentication& message,
+ base::OnceClosure resume_callback) override {
+ authenticator_->ProcessMessage(message, std::move(resume_callback));
+ }
+ JingleAuthentication GetNextMessage() override {
+ return authenticator_->GetNextMessage();
+ }
+ const std::string& GetAuthKey() const override {
+ return authenticator_->GetAuthKey();
+ }
+ const SessionPolicies* GetSessionPolicies() const override {
+ return authenticator_->GetSessionPolicies();
+ }
+ std::unique_ptr<ChannelAuthenticator> CreateChannelAuthenticator()
+ const override {
+ return authenticator_->CreateChannelAuthenticator();
+ }
+
+ private:
+ raw_ptr<Authenticator> authenticator_;
+};
+
} // namespace
class NegotiatingAuthenticatorTest : public AuthenticatorTestBase {
@@ -176,6 +237,19 @@
raw_ptr<NegotiatingHostAuthenticator> host_as_negotiating_authenticator_;
raw_ptr<NegotiatingClientAuthenticator> client_as_negotiating_authenticator_;
+ protected:
+ void SetHostState(Authenticator::State state) {
+ host_as_negotiating_authenticator_->state_ = state;
+ }
+
+ void SetHostCurrentMethod(AuthenticationMethod method) {
+ host_as_negotiating_authenticator_->current_method_ = method;
+ }
+
+ void NotifyHostStateChangeAfterAccepted() {
+ host_as_negotiating_authenticator_->NotifyStateChangeAfterAccepted();
+ }
+
private:
scoped_refptr<PairingRegistry> pairing_registry_;
};
@@ -333,4 +407,57 @@
ASSERT_NE(&host_->implementing_authenticator(), host_.get());
}
+TEST_F(NegotiatingAuthenticatorTest, GetNextMessage_SynchronousTeardown) {
+ auto auth_config =
+ std::make_unique<HostAuthenticationConfig>(host_cert_, key_pair_);
+ auth_config->AddSharedSecretAuth("hash");
+ auto host = std::make_unique<TestNegotiatingHostAuthenticator>(
+ kHostJid, kClientJid, std::move(auth_config));
+ TestNegotiatingHostAuthenticator* host_ptr = host.get();
+
+ // Setup mock underlying authenticator.
+ auto mock_host_authenticator =
+ std::make_unique<testing::NiceMock<MockAuthenticator>>();
+
+ // Use a ProxyAuthenticator so the mock outlives the host.
+ host_ptr->current_authenticator_ =
+ std::make_unique<ProxyAuthenticator>(mock_host_authenticator.get());
+ host_ptr->ChainStateChangeAfterAcceptedWithUnderlying(
+ *host_ptr->current_authenticator_);
+
+ // Set host state to MESSAGE_READY.
+ host_ptr->state_ = Authenticator::MESSAGE_READY;
+ host_ptr->current_method_ =
+ AuthenticationMethod::SHARED_SECRET_SPAKE2_CURVE25519;
+
+ base::WeakPtr<NegotiatingAuthenticatorBase> host_weak = host->GetWeakPtr();
+
+ EXPECT_CALL(*mock_host_authenticator, state())
+ .WillRepeatedly(Return(Authenticator::MESSAGE_READY));
+
+ // When GetNextMessage() is called on mock_host_authenticator, we transition
+ // its state to REJECTED and notify teardown.
+ EXPECT_CALL(*mock_host_authenticator, GetNextMessage()).WillOnce([&]() {
+ EXPECT_CALL(*mock_host_authenticator, state())
+ .WillRepeatedly(Return(Authenticator::REJECTED));
+ if (host_weak) {
+ static_cast<TestNegotiatingHostAuthenticator*>(host_weak.get())
+ ->NotifyStateChangeAfterAcceptedForTesting();
+ }
+ return JingleAuthentication();
+ });
+
+ // Set the state change callback to destroy host.
+ host_ptr->set_state_change_after_accepted_callback(base::BindRepeating(
+ [](std::unique_ptr<TestNegotiatingHostAuthenticator>* host) {
+ host->reset();
+ },
+ &host));
+
+ // This should NOT trigger UAF because of the WeakPtr check.
+ std::ignore = host_ptr->GetNextMessage();
+
+ ASSERT_EQ(host, nullptr);
+}
+
} // namespace remoting::protocol
Original Bug Report
Potential Use-After-Free in SessionAuthzAuthenticator via synchronous teardown
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the Chrome Security team.
Overview: A potential Use-After-Free (UAF) vulnerability exists in the Chrome Remote Desktop host process. An invalid SPAKE2 message triggers a synchronous teardown of the session, destroying the authenticator tree while it is still on the call stack. Execution then resumes on the freed object, leading to a virtual method call on freed memory that could be exploited for Remote Code Execution.
Affected files:
remoting/protocol/session_authz_authenticator.ccremoting/protocol/spake2_authenticator.ccremoting/host/chromoting_host.ccremoting/protocol/jingle_session.ccremoting/host/client_session.cc
Estimated timestamp from git blame: 2026-02-12
Description
A potential Use-After-Free (UAF) vulnerability exists in the remoting::protocol::SessionAuthzAuthenticator class used by the Chrome Remote Desktop host (remoting_me2me_host).
The issue occurs because SessionAuthzAuthenticator delegates message processing to an underlying authenticator (typically Spake2Authenticator) and passes a continuation callback. If the underlying authentication fails, this callback is executed synchronously and initiates a complete teardown of the session. Because the session hierarchy is managed via std::unique_ptrs, the SessionAuthzAuthenticator itself is synchronously destroyed. However, after the callback returns, execution resumes in SessionAuthzAuthenticator where it accesses the now-freed this pointer to execute StartReauthorizerIfNecessary().
Inside StartReauthorizerIfNecessary(), the code accesses freed std::unique_ptr members and makes a virtual method call (underlying_->state()). Since this occurs in the unsandboxed host process, an attacker who reallocates the freed memory could hijack the virtual method table to achieve Remote Code Execution (RCE).
This vulnerability is not mitigated by MiraclePtr (BackupRefPtr) because the dangling pointer is the implicit this pointer on the call stack, and the internal members are held via standard std::unique_ptr rather than raw_ptr.
Note: The following steps are suggested/potential paths to trigger the vulnerability, as our analysis tooling does not currently have the ability to run code and provide a live proof-of-concept.
Suggested Attacker Steps
- The attacker initiates a Chrome Remote Desktop connection to a target host, obtaining a valid
SessionAuthzsession token (requiring some level of authorized identity). - During the negotiation phase, the attacker deliberately sends an invalid or malformed SPAKE2 message (e.g., an incorrect verification hash) alongside the valid session token.
SessionAuthzAuthenticator::ProcessMessage(orOnVerifiedSessionToken) executes and callsunderlying_->ProcessMessage(message, std::move(resume_callback));.- The
Spake2Authenticatorprocesses the message, detects the invalid payload, and transitions its state toREJECTED. Spake2Authenticatorsynchronously executes theresume_callback.- The callback propagates to
JingleSession::ProcessAuthenticationStep, which observes theREJECTEDstate and callsJingleSession::Close(). Close()triggers a synchronous state change toFAILED, which notifiesWebrtcConnectionToClient, which notifiesClientSession, which finally notifiesChromotingHost.ChromotingHost::OnSessionClosedsynchronously erases theClientSessionfrom itsclients_multimap.- This erasure cascades down, synchronously destroying the
ClientSession,WebrtcConnectionToClient,JingleSession,NegotiatingHostAuthenticator, and finally theSessionAuthzAuthenticator. - Execution unwinds and returns to
SessionAuthzAuthenticator::ProcessMessage, whereStartReauthorizerIfNecessary()is immediately invoked on the freedthispointer. - The virtual call
underlying_->state()uses the freed memory, providing a vtable hijack primitive if the attacker successfully manipulated the heap in the interim.
Suggested Fix
To safely handle synchronous destruction, the SessionAuthzAuthenticator should avoid accessing this after delegating to the underlying authenticator.
A common pattern to fix this is to bind the subsequent work (StartReauthorizerIfNecessary) to a base::WeakPtr and invoke it asynchronously via base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(...), or use a base::WeakPtr check immediately after underlying_->ProcessMessage returns before proceeding with any state access.
Evaluated with Chrome root at commit: 096fc8fdbfacf2546485756d03f160a3d04fcc9b
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.