From 7f7eb9ebf9e60c8d513311e2f14c3c4366825291 Mon Sep 17 00:00:00 2001
From: Joe Downing <joedow@google.com>
Date: Wed, 09 Sep 2026 11:32:39 -0700
Subject: [PATCH] Validate WTS terminal IDs in the Windows remoting host

In the Windows remoting host, WtsTerminalMonitor identifies WTS sessions
using terminal ID strings. For sessions not attached to the physical
console, the ID is obtained from the session's initial-program working
directory provided by the RDP connection. Previously, this value was
accepted without validation, allowing unexpected strings—such as reserved console identifiers or empty strings—to be treated as valid
terminal IDs.

This change introduces format validation for virtual terminal IDs in
WtsTerminalMonitor via GenerateVirtualTerminalId() and
IsVirtualTerminalId(), ensuring that only well-formed virtual terminal
IDs are accepted during lookups. Additionally,
HostService::OnSessionChange() is updated so that an observer already
bound to a session is only re-attached if it monitors the console
terminal, preserving session bindings for virtual terminals.

TAG=workflow
TAG=agy
CONV=5cfca178-d091-43e2-8570-edfe330ad1c8

Bug: 553154579
Change-Id: Ie8a6196ab4aee39945e34f525e157e43bbf3dc67
Fixed: 553154579
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8373171
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1694764}
---
diff --git a/remoting/host/desktop_session_win.cc b/remoting/host/desktop_session_win.cc
index 375072a..ae53a29 100644
--- a/remoting/host/desktop_session_win.cc
+++ b/remoting/host/desktop_session_win.cc
@@ -26,7 +26,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_checker.h"
 #include "base/timer/timer.h"
-#include "base/uuid.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_bstr.h"
 #include "base/win/scoped_handle.h"
@@ -320,7 +319,7 @@
   // Create an RDP session.
   Microsoft::WRL::ComPtr<IRdpDesktopSessionEventHandler> event_handler(
       new EventHandler(weak_factory_.GetWeakPtr()));
-  terminal_id_ = base::Uuid::GenerateRandomV4().AsLowercaseString();
+  terminal_id_ = WtsTerminalMonitor::GenerateVirtualTerminalId();
   base::win::ScopedBstr terminal_id(base::UTF8ToWide(terminal_id_));
   result = rdp_desktop_session_->Connect(
       host_size.width(), host_size.height(), kDefaultRdpDpi, kDefaultRdpDpi,
diff --git a/remoting/host/win/host_service.cc b/remoting/host/win/host_service.cc
index 2ebc17ba..d394a71 100644
--- a/remoting/host/win/host_service.cc
+++ b/remoting/host/win/host_service.cc
@@ -224,10 +224,16 @@
       continue;
     }
 
-    // The client currently attached to |session_id| was attached to a different
-    // session before. Reconnect it to |session_id|.
+    // The terminal currently attached to |session_id| was attached to a
+    // different session before. Reconnect it to |session_id|. Only the console
+    // terminal may be re-attached while its observer is still bound to another
+    // session: the physical console moves between sessions (e.g. on fast user
+    // switching). A virtual terminal is backed by a single RDP connection, so
+    // its observer stays bound to the session the terminal was originally
+    // attached to.
     if (attached && i->terminal_id == terminal_id &&
-        i->session_id != session_id) {
+        i->session_id != session_id &&
+        (i->session_id == kInvalidSessionId || i->terminal_id == kConsole)) {
       WtsTerminalObserver* observer = i->observer;
 
       if (i->session_id != kInvalidSessionId) {
diff --git a/remoting/host/win/wts_terminal_monitor.cc b/remoting/host/win/wts_terminal_monitor.cc
index 4b25ab6..5eb4f30 100644
--- a/remoting/host/win/wts_terminal_monitor.cc
+++ b/remoting/host/win/wts_terminal_monitor.cc
@@ -13,6 +13,7 @@
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/uuid.h"
 #include "third_party/abseil-cpp/absl/cleanup/cleanup.h"
 
 namespace remoting {
@@ -46,8 +47,23 @@
   absl::Cleanup wts_deleter = [working_directory] {
     ::WTSFreeMemory(working_directory);
   };
-  return base::WideToUTF8(working_directory, (bytes / sizeof(wchar_t)) - 1,
-                          terminal_id);
+
+  std::string id;
+  if (!base::WideToUTF8(working_directory, (bytes / sizeof(wchar_t)) - 1,
+                        &id)) {
+    return false;
+  }
+
+  // The working directory is controlled by the client of the RDP connection,
+  // so it identifies a terminal only if it carries a virtual terminal ID
+  // assigned by RdpClient. In particular, a session that is not attached to
+  // the physical console must never be reported as the console terminal.
+  if (!IsVirtualTerminalId(id)) {
+    return false;
+  }
+
+  *terminal_id = std::move(id);
+  return true;
 }
 
 // static
@@ -81,6 +97,16 @@
   return kInvalidSessionId;
 }
 
+// static
+std::string WtsTerminalMonitor::GenerateVirtualTerminalId() {
+  return base::Uuid::GenerateRandomV4().AsLowercaseString();
+}
+
+// static
+bool WtsTerminalMonitor::IsVirtualTerminalId(const std::string& terminal_id) {
+  return base::Uuid::ParseLowercase(terminal_id).is_valid();
+}
+
 WtsTerminalMonitor::WtsTerminalMonitor() = default;
 
 }  // namespace remoting
diff --git a/remoting/host/win/wts_terminal_monitor.h b/remoting/host/win/wts_terminal_monitor.h
index d91e0d5..8e5b383d3 100644
--- a/remoting/host/win/wts_terminal_monitor.h
+++ b/remoting/host/win/wts_terminal_monitor.h
@@ -42,8 +42,10 @@
   virtual void RemoveWtsTerminalObserver(WtsTerminalObserver* observer) = 0;
 
   // Returns ID of the terminal connected to |session_id| in |*terminal_id|.
-  // Returns false if |session_id| is not attached to the physical console or
-  // does not have an assigned terminal ID.
+  // Returns false if |session_id| does not have an assigned terminal ID.
+  // |kConsole| is only ever returned for the session that is attached to the
+  // physical console; any other session is identified by the virtual terminal
+  // ID that RdpClient assigned to it.
   static bool LookupTerminalId(uint32_t session_id, std::string* terminal_id);
 
   // Returns ID of the session that |terminal_id| is attached.
@@ -51,6 +53,16 @@
   // attahced to |client_endpoint|.
   static uint32_t LookupSessionId(const std::string& terminal_id);
 
+  // Generates a unique ID for a new virtual terminal, i.e. a terminal backed
+  // by an RDP connection created by RdpClient.
+  static std::string GenerateVirtualTerminalId();
+
+  // Returns true if |terminal_id| is a well-formed virtual terminal ID as
+  // produced by GenerateVirtualTerminalId(). |kConsole| is not a virtual
+  // terminal ID: it always identifies the terminal attached to the physical
+  // console.
+  static bool IsVirtualTerminalId(const std::string& terminal_id);
+
  protected:
   WtsTerminalMonitor();
 };
diff --git a/remoting/host/win/rdp_client_unittest.cc b/remoting/host/win/rdp_client_unittest.cc
index bd91c40e..01e59c5 100644
--- a/remoting/host/win/rdp_client_unittest.cc
+++ b/remoting/host/win/rdp_client_unittest.cc
@@ -13,7 +13,6 @@
 #include "base/run_loop.h"
 #include "base/task/single_thread_task_runner.h"
 #include "base/test/task_environment.h"
-#include "base/uuid.h"
 #include "base/win/atl.h"
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/windows_version.h"
@@ -167,7 +166,7 @@
     GTEST_SKIP() << "https://crbug.com/365126540: Skipping test for WIN11_23H2 "
                     "and greater";
   }
-  terminal_id_ = base::Uuid::GenerateRandomV4().AsLowercaseString();
+  terminal_id_ = WtsTerminalMonitor::GenerateVirtualTerminalId();
 
   // An ability to establish a loopback RDP connection depends on many factors
   // including OS SKU and having RDP enabled. Accept both successful connection
diff --git a/remoting/host/win/wts_terminal_monitor_unittest.cc b/remoting/host/win/wts_terminal_monitor_unittest.cc
new file mode 100644
index 0000000..8dd6213
--- /dev/null
+++ b/remoting/host/win/wts_terminal_monitor_unittest.cc
@@ -0,0 +1,135 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "remoting/host/win/wts_terminal_monitor.h"
+
+#include <windows.h>
+
+#include <wtsapi32.h>
+
+#include <string>
+
+#include "base/compiler_specific.h"
+#include "base/containers/span.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/abseil-cpp/absl/cleanup/cleanup.h"
+
+namespace remoting {
+
+TEST(WtsTerminalMonitorTest, GeneratedVirtualTerminalIdsAreValid) {
+  EXPECT_TRUE(WtsTerminalMonitor::IsVirtualTerminalId(
+      WtsTerminalMonitor::GenerateVirtualTerminalId()));
+}
+
+TEST(WtsTerminalMonitorTest, GeneratedVirtualTerminalIdsAreUnique) {
+  EXPECT_NE(WtsTerminalMonitor::GenerateVirtualTerminalId(),
+            WtsTerminalMonitor::GenerateVirtualTerminalId());
+}
+
+// The console terminal ID identifies the session that is attached to the
+// physical console, so it must never be accepted as a virtual terminal ID.
+TEST(WtsTerminalMonitorTest, ConsoleTerminalIdIsNotAVirtualTerminalId) {
+  EXPECT_FALSE(
+      WtsTerminalMonitor::IsVirtualTerminalId(WtsTerminalMonitor::kConsole));
+}
+
+// Virtual terminal IDs are read back from the working directory that RdpClient
+// specifies when connecting. Working directory values that RdpClient does not
+// produce do not identify a virtual terminal.
+class WtsTerminalMonitorInvalidIdTest
+    : public testing::TestWithParam<const char*> {};
+
+TEST_P(WtsTerminalMonitorInvalidIdTest, IsNotVirtualTerminalId) {
+  EXPECT_FALSE(WtsTerminalMonitor::IsVirtualTerminalId(GetParam()));
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    InvalidVirtualTerminalIds,
+    WtsTerminalMonitorInvalidIdTest,
+    testing::Values("",
+                    "Console",
+                    "C:\\Users\\Default",
+                    "%HOMEDRIVE%%HOMEPATH%\\console",
+                    // Uppercase UUIDs are rejected.
+                    "D3B07384-D113-467F-94D7-E07E868A8677",
+                    // Braced UUIDs are rejected.
+                    "{d3b07384-d113-467f-94d7-e07e868a8677}",
+                    // UUIDs without hyphens are rejected.
+                    "d3b07384d113467f94d7e07e868a8677",
+                    // UUIDs with leading or trailing whitespace are rejected.
+                    " d3b07384-d113-467f-94d7-e07e868a8677",
+                    "d3b07384-d113-467f-94d7-e07e868a8677 "));
+
+// The session attached to the physical console is reported as the console
+// terminal.
+TEST(WtsTerminalMonitorTest, ActiveConsoleSessionMapsToConsoleTerminal) {
+  uint32_t console_session_id = WTSGetActiveConsoleSessionId();
+  // WTSGetActiveConsoleSessionId() returns the ID of the session attached to
+  // the physical console. This is expected to execute on CQ try-bots and
+  // during local interactive desktop runs. However, it returns
+  // kInvalidSessionId when executed within an RDP session or in a headless
+  // environment without an attached physical console, in which case the test
+  // is skipped.
+  if (console_session_id == kInvalidSessionId) {
+    GTEST_SKIP() << "No session is attached to the physical console.";
+  }
+
+  std::string terminal_id;
+  ASSERT_TRUE(
+      WtsTerminalMonitor::LookupTerminalId(console_session_id, &terminal_id));
+  EXPECT_EQ(terminal_id, WtsTerminalMonitor::kConsole);
+}
+
+// The console terminal always resolves to the session attached to the physical
+// console.
+TEST(WtsTerminalMonitorTest, ConsoleTerminalMapsToActiveConsoleSession) {
+  EXPECT_EQ(WtsTerminalMonitor::LookupSessionId(WtsTerminalMonitor::kConsole),
+            WTSGetActiveConsoleSessionId());
+}
+
+// A session that is not attached to the physical console must never be
+// reported as the console terminal, and may only be identified by the virtual
+// terminal ID assigned to it.
+TEST(WtsTerminalMonitorTest, NonConsoleSessionsDoNotMapToConsoleTerminal) {
+  uint32_t console_session_id = WTSGetActiveConsoleSessionId();
+
+  WTS_SESSION_INFO* session_info;
+  DWORD session_info_count;
+  ASSERT_TRUE(WTSEnumerateSessions(WTS_CURRENT_SERVER_HANDLE, 0, 1,
+                                   &session_info, &session_info_count));
+  absl::Cleanup wts_deleter = [session_info] { ::WTSFreeMemory(session_info); };
+
+  // SAFETY: WTSEnumerateSessions() allocated `session_info_count` entries.
+  auto sessions = UNSAFE_BUFFERS(
+      base::span<const WTS_SESSION_INFO>(session_info, session_info_count));
+  for (const WTS_SESSION_INFO& session : sessions) {
+    if (session.SessionId == console_session_id) {
+      continue;
+    }
+
+    std::string terminal_id;
+    if (WtsTerminalMonitor::LookupTerminalId(session.SessionId, &terminal_id)) {
+      EXPECT_TRUE(WtsTerminalMonitor::IsVirtualTerminalId(terminal_id))
+          << "Session " << session.SessionId
+          << " reported terminal ID: " << terminal_id;
+    }
+  }
+}
+
+// A virtual terminal that no RDP connection was created for is not associated
+// with any session.
+TEST(WtsTerminalMonitorTest, UnknownVirtualTerminalIdMapsToNoSession) {
+  EXPECT_EQ(WtsTerminalMonitor::LookupSessionId(
+                WtsTerminalMonitor::GenerateVirtualTerminalId()),
+            kInvalidSessionId);
+}
+
+// The empty string is not a terminal ID, so it must not resolve to a session
+// even if a session reports an empty working directory.
+TEST(WtsTerminalMonitorTest, EmptyTerminalIdMapsToNoSession) {
+  EXPECT_EQ(WtsTerminalMonitor::LookupSessionId(std::string()),
+            kInvalidSessionId);
+}
+
+}  // namespace remoting
diff --git a/remoting/host/win/BUILD.gn b/remoting/host/win/BUILD.gn
index 03092f6..bc653cb 100644
--- a/remoting/host/win/BUILD.gn
+++ b/remoting/host/win/BUILD.gn
@@ -236,6 +236,7 @@
     "rdp_client_unittest.cc",
     "unprivileged_process_delegate_unittest.cc",
     "wts_session_process_delegate_unittest.cc",
+    "wts_terminal_monitor_unittest.cc",
   ]
 
   deps = [

diff --git a/remoting/host/win/rdp_client_unittest.cc b/remoting/host/win/rdp_client_unittest.cc
index bd91c40e..01e59c5 100644
--- a/remoting/host/win/rdp_client_unittest.cc
+++ b/remoting/host/win/rdp_client_unittest.cc
@@ -13,7 +13,6 @@
 #include "base/run_loop.h"
 #include "base/task/single_thread_task_runner.h"
 #include "base/test/task_environment.h"
-#include "base/uuid.h"
 #include "base/win/atl.h"
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/windows_version.h"
@@ -167,7 +166,7 @@
     GTEST_SKIP() << "https://crbug.com/365126540: Skipping test for WIN11_23H2 "
                     "and greater";
   }
-  terminal_id_ = base::Uuid::GenerateRandomV4().AsLowercaseString();
+  terminal_id_ = WtsTerminalMonitor::GenerateVirtualTerminalId();
 
   // An ability to establish a loopback RDP connection depends on many factors
   // including OS SKU and having RDP enabled. Accept both successful connection
diff --git a/remoting/host/win/wts_terminal_monitor_unittest.cc b/remoting/host/win/wts_terminal_monitor_unittest.cc
new file mode 100644
index 0000000..8dd6213
--- /dev/null
+++ b/remoting/host/win/wts_terminal_monitor_unittest.cc
@@ -0,0 +1,135 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "remoting/host/win/wts_terminal_monitor.h"
+
+#include <windows.h>
+
+#include <wtsapi32.h>
+
+#include <string>
+
+#include "base/compiler_specific.h"
+#include "base/containers/span.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/abseil-cpp/absl/cleanup/cleanup.h"
+
+namespace remoting {
+
+TEST(WtsTerminalMonitorTest, GeneratedVirtualTerminalIdsAreValid) {
+  EXPECT_TRUE(WtsTerminalMonitor::IsVirtualTerminalId(
+      WtsTerminalMonitor::GenerateVirtualTerminalId()));
+}
+
+TEST(WtsTerminalMonitorTest, GeneratedVirtualTerminalIdsAreUnique) {
+  EXPECT_NE(WtsTerminalMonitor::GenerateVirtualTerminalId(),
+            WtsTerminalMonitor::GenerateVirtualTerminalId());
+}
+
+// The console terminal ID identifies the session that is attached to the
+// physical console, so it must never be accepted as a virtual terminal ID.
+TEST(WtsTerminalMonitorTest, ConsoleTerminalIdIsNotAVirtualTerminalId) {
+  EXPECT_FALSE(
+      WtsTerminalMonitor::IsVirtualTerminalId(WtsTerminalMonitor::kConsole));
+}
+
+// Virtual terminal IDs are read back from the working directory that RdpClient
+// specifies when connecting. Working directory values that RdpClient does not
+// produce do not identify a virtual terminal.
+class WtsTerminalMonitorInvalidIdTest
+    : public testing::TestWithParam<const char*> {};
+
+TEST_P(WtsTerminalMonitorInvalidIdTest, IsNotVirtualTerminalId) {
+  EXPECT_FALSE(WtsTerminalMonitor::IsVirtualTerminalId(GetParam()));
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    InvalidVirtualTerminalIds,
+    WtsTerminalMonitorInvalidIdTest,
+    testing::Values("",
+                    "Console",
+                    "C:\\Users\\Default",
+                    "%HOMEDRIVE%%HOMEPATH%\\console",
+                    // Uppercase UUIDs are rejected.
+                    "D3B07384-D113-467F-94D7-E07E868A8677",
+                    // Braced UUIDs are rejected.
+                    "{d3b07384-d113-467f-94d7-e07e868a8677}",
+                    // UUIDs without hyphens are rejected.
+                    "d3b07384d113467f94d7e07e868a8677",
+                    // UUIDs with leading or trailing whitespace are rejected.
+                    " d3b07384-d113-467f-94d7-e07e868a8677",
+                    "d3b07384-d113-467f-94d7-e07e868a8677 "));
+
+// The session attached to the physical console is reported as the console
+// terminal.
+TEST(WtsTerminalMonitorTest, ActiveConsoleSessionMapsToConsoleTerminal) {
+  uint32_t console_session_id = WTSGetActiveConsoleSessionId();
+  // WTSGetActiveConsoleSessionId() returns the ID of the session attached to
+  // the physical console. This is expected to execute on CQ try-bots and
+  // during local interactive desktop runs. However, it returns
+  // kInvalidSessionId when executed within an RDP session or in a headless
+  // environment without an attached physical console, in which case the test
+  // is skipped.
+  if (console_session_id == kInvalidSessionId) {
+    GTEST_SKIP() << "No session is attached to the physical console.";
+  }
+
+  std::string terminal_id;
+  ASSERT_TRUE(
+      WtsTerminalMonitor::LookupTerminalId(console_session_id, &terminal_id));
+  EXPECT_EQ(terminal_id, WtsTerminalMonitor::kConsole);
+}
+
+// The console terminal always resolves to the session attached to the physical
+// console.
+TEST(WtsTerminalMonitorTest, ConsoleTerminalMapsToActiveConsoleSession) {
+  EXPECT_EQ(WtsTerminalMonitor::LookupSessionId(WtsTerminalMonitor::kConsole),
+            WTSGetActiveConsoleSessionId());
+}
+
+// A session that is not attached to the physical console must never be
+// reported as the console terminal, and may only be identified by the virtual
+// terminal ID assigned to it.
+TEST(WtsTerminalMonitorTest, NonConsoleSessionsDoNotMapToConsoleTerminal) {
+  uint32_t console_session_id = WTSGetActiveConsoleSessionId();
+
+  WTS_SESSION_INFO* session_info;
+  DWORD session_info_count;
+  ASSERT_TRUE(WTSEnumerateSessions(WTS_CURRENT_SERVER_HANDLE, 0, 1,
+                                   &session_info, &session_info_count));
+  absl::Cleanup wts_deleter = [session_info] { ::WTSFreeMemory(session_info); };
+
+  // SAFETY: WTSEnumerateSessions() allocated `session_info_count` entries.
+  auto sessions = UNSAFE_BUFFERS(
+      base::span<const WTS_SESSION_INFO>(session_info, session_info_count));
+  for (const WTS_SESSION_INFO& session : sessions) {
+    if (session.SessionId == console_session_id) {
+      continue;
+    }
+
+    std::string terminal_id;
+    if (WtsTerminalMonitor::LookupTerminalId(session.SessionId, &terminal_id)) {
+      EXPECT_TRUE(WtsTerminalMonitor::IsVirtualTerminalId(terminal_id))
+          << "Session " << session.SessionId
+          << " reported terminal ID: " << terminal_id;
+    }
+  }
+}
+
+// A virtual terminal that no RDP connection was created for is not associated
+// with any session.
+TEST(WtsTerminalMonitorTest, UnknownVirtualTerminalIdMapsToNoSession) {
+  EXPECT_EQ(WtsTerminalMonitor::LookupSessionId(
+                WtsTerminalMonitor::GenerateVirtualTerminalId()),
+            kInvalidSessionId);
+}
+
+// The empty string is not a terminal ID, so it must not resolve to a session
+// even if a session reports an empty working directory.
+TEST(WtsTerminalMonitorTest, EmptyTerminalIdMapsToNoSession) {
+  EXPECT_EQ(WtsTerminalMonitor::LookupSessionId(std::string()),
+            kInvalidSessionId);
+}
+
+}  // namespace remoting