Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactUse after free in Chromoting
DescriptionUse after free in Chromoting
ComponentChromoting
Bug ClassUAF
Tracker501376612
Fix commit174583c9fcf8 (chromium/src) +155/-7
CISA KEVNot listed
CreditedGoogle
Disclosed2026-06-02

Changed Functions

FunctionChangeNotes
if
remoting/protocol/negotiating_authenticator_base.cc
modified
TestNegotiatingHostAuthenticator
remoting/protocol/negotiating_authenticator_unittest.cc
modified
ProxyAuthenticator
remoting/protocol/negotiating_authenticator_unittest.cc
modified
NegotiatingAuthenticatorTest
remoting/protocol/negotiating_authenticator_unittest.cc
modified

Files Changed

  • remoting/protocol/authenticator.cc
  • remoting/protocol/authenticator.h
  • remoting/protocol/negotiating_authenticator_base.cc
  • remoting/protocol/negotiating_authenticator_base.h
  • remoting/protocol/negotiating_authenticator_unittest.cc
From 174583c9fcf83d5f5b5d1abc841366f6c8986556 Mon Sep 17 00:00:00 2001
From: Joe Downing <joedow@google.com>
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 <yuweih@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
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() {
Loading diff…

Regression Test / PoC

shipped with the fix
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
Loading diff…

Original Bug Report

reported by vm...@google.com

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.cc
  • remoting/protocol/spake2_authenticator.cc
  • remoting/host/chromoting_host.cc
  • remoting/protocol/jingle_session.cc
  • remoting/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

  1. The attacker initiates a Chrome Remote Desktop connection to a target host, obtaining a valid SessionAuthz session token (requiring some level of authorized identity).
  2. 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.
  3. SessionAuthzAuthenticator::ProcessMessage (or OnVerifiedSessionToken) executes and calls underlying_->ProcessMessage(message, std::move(resume_callback));.
  4. The Spake2Authenticator processes the message, detects the invalid payload, and transitions its state to REJECTED.
  5. Spake2Authenticator synchronously executes the resume_callback.
  6. The callback propagates to JingleSession::ProcessAuthenticationStep, which observes the REJECTED state and calls JingleSession::Close().
  7. Close() triggers a synchronous state change to FAILED, which notifies WebrtcConnectionToClient, which notifies ClientSession, which finally notifies ChromotingHost.
  8. ChromotingHost::OnSessionClosed synchronously erases the ClientSession from its clients_ multimap.
  9. This erasure cascades down, synchronously destroying the ClientSession, WebrtcConnectionToClient, JingleSession, NegotiatingHostAuthenticator, and finally the SessionAuthzAuthenticator.
  10. Execution unwinds and returns to SessionAuthzAuthenticator::ProcessMessage, where StartReauthorizerIfNecessary() is immediately invoked on the freed this pointer.
  11. 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.

View on issue tracker