From b0ac65436bc652d8a5d2c91115ba28c60654d4f9 Mon Sep 17 00:00:00 2001
From: Yuwei Huang <yuweih@chromium.org>
Date: Tue, 07 Jul 2026 16:30:56 -0700
Subject: [PATCH] [remoting][win] Identify WebAuthn NMH launcher via stdio pipes

We previously tried to identify the real NMH launcher by checking if the
parent is CMD then looking at its grandparent.

It turns out Chrome actually creates a pair of named pipes, namely
`\\.\pipe\chrome.nativeMessaging.in.<token>` and
`\\.\pipe\chrome.nativeMessaging.out.<token>`, then passes them to the
NMH via the cmd command
`cmd.exe /d /s /c $COMMAND < $IN_PIPE > $OUT_PIPE`, which means the
named pipes are directly forwarded to the NMH.

It is much easier and safer to just identify the server PID of the named
pipes than trying to figure out the what the grandparent process is, so
this CL implements the new approach.

Bug: 518053893
Change-Id: I50fb1cb3731f508d058a63baa4864aacb62bf330
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8057588
Auto-Submit: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1658366}
---
diff --git a/remoting/host/base/process_util.cc b/remoting/host/base/process_util.cc
index 3302774..592c742 100644
--- a/remoting/host/base/process_util.cc
+++ b/remoting/host/base/process_util.cc
@@ -83,4 +83,45 @@
 #endif
 }
 
+#if BUILDFLAG(IS_WIN)
+base::ProcessId GetLauncherProcessIdFromPipes(HANDLE stdin_handle,
+                                              HANDLE stdout_handle) {
+  if (stdin_handle == INVALID_HANDLE_VALUE || stdin_handle == nullptr ||
+      stdout_handle == INVALID_HANDLE_VALUE || stdout_handle == nullptr) {
+    return base::kNullProcessId;
+  }
+
+  if (::GetFileType(stdin_handle) != FILE_TYPE_PIPE ||
+      ::GetFileType(stdout_handle) != FILE_TYPE_PIPE) {
+    return base::kNullProcessId;
+  }
+
+  ULONG stdin_server_pid = 0;
+  if (!::GetNamedPipeServerProcessId(stdin_handle, &stdin_server_pid)) {
+    PLOG(ERROR) << "GetNamedPipeServerProcessId failed for stdin";
+    return base::kNullProcessId;
+  }
+
+  ULONG stdout_server_pid = 0;
+  if (!::GetNamedPipeServerProcessId(stdout_handle, &stdout_server_pid)) {
+    PLOG(ERROR) << "GetNamedPipeServerProcessId failed for stdout";
+    return base::kNullProcessId;
+  }
+
+  if (stdin_server_pid != stdout_server_pid) {
+    LOG(ERROR)
+        << "stdin and stdout pipes belong to different server processes ("
+        << stdin_server_pid << " vs " << stdout_server_pid << ")";
+    return base::kNullProcessId;
+  }
+
+  return static_cast<base::ProcessId>(stdin_server_pid);
+}
+
+base::ProcessId GetLauncherProcessIdFromStdioPipes() {
+  return GetLauncherProcessIdFromPipes(::GetStdHandle(STD_INPUT_HANDLE),
+                                       ::GetStdHandle(STD_OUTPUT_HANDLE));
+}
+#endif
+
 }  // namespace remoting
diff --git a/remoting/host/base/process_util.h b/remoting/host/base/process_util.h
index 8f6367b..86fb48e5 100644
--- a/remoting/host/base/process_util.h
+++ b/remoting/host/base/process_util.h
@@ -8,6 +8,11 @@
 #include "base/files/file_path.h"
 #include "base/process/process.h"
 #include "base/process/process_handle.h"
+#include "build/build_config.h"
+
+#if BUILDFLAG(IS_WIN)
+#include "base/win/windows_types.h"
+#endif
 
 namespace remoting {
 
@@ -19,6 +24,19 @@
 // Same as above but using an existing process handle.
 base::FilePath GetProcessImagePath(const base::Process& process);
 
+#if BUILDFLAG(IS_WIN)
+// Returns the process ID of the named-pipe server connected to the specified
+// input and output pipe handles. Returns base::kNullProcessId if either handle
+// is invalid, not a named pipe, or connected to different named-pipe servers.
+base::ProcessId GetLauncherProcessIdFromPipes(HANDLE stdin_handle,
+                                              HANDLE stdout_handle);
+
+// Returns the process ID of the named-pipe server connected to standard input
+// and standard output. Returns base::kNullProcessId if stdin or stdout is not
+// a named pipe, or if they are connected to different named-pipe servers.
+base::ProcessId GetLauncherProcessIdFromStdioPipes();
+#endif
+
 }  // namespace remoting
 
 #endif  // REMOTING_HOST_BASE_PROCESS_UTIL_H_
diff --git a/remoting/host/webauthn/remote_webauthn_caller_security_utils.cc b/remoting/host/webauthn/remote_webauthn_caller_security_utils.cc
index 91a2b288..be14b2c 100644
--- a/remoting/host/webauthn/remote_webauthn_caller_security_utils.cc
+++ b/remoting/host/webauthn/remote_webauthn_caller_security_utils.cc
@@ -58,39 +58,6 @@
 
 #elif BUILDFLAG(IS_WIN)
 
-// Helper to verify that the cmd.exe binary is located in a trusted system
-// directory.
-bool IsSystemCmd(const base::FilePath& process_image_path) {
-  if (!base::FilePath::CompareEqualIgnoreCase(
-          process_image_path.BaseName().value(), L"cmd.exe")) {
-    return false;
-  }
-
-  base::FilePath process_dir = process_image_path.DirName();
-
-  base::FilePath system_dir;
-  if (base::PathService::Get(base::DIR_SYSTEM, &system_dir)) {
-    if (base::FilePath::CompareEqualIgnoreCase(process_dir.value(),
-                                               system_dir.value())) {
-      return true;
-    }
-  }
-
-  // Also check for the native system directory if it's different (e.g.
-  // SysWOW64 vs System32).
-  base::FilePath windows_dir;
-  if (base::PathService::Get(base::DIR_WINDOWS, &windows_dir)) {
-    if (base::FilePath::CompareEqualIgnoreCase(
-            process_dir.value(), windows_dir.Append(L"System32").value()) ||
-        base::FilePath::CompareEqualIgnoreCase(
-            process_dir.value(), windows_dir.Append(L"SysWOW64").value())) {
-      return true;
-    }
-  }
-
-  return false;
-}
-
 #elif BUILDFLAG(IS_MAC)
 
 constexpr auto kAllowedIdentifiers = std::to_array<const std::string_view>(
@@ -115,35 +82,19 @@
   base::FilePath parent_image_path = GetProcessImagePath(parent_pid);
   return kAllowedCallerPrograms.contains(parent_image_path.value());
 #elif BUILDFLAG(IS_WIN)
-  base::ProcessId parent_pid =
-      base::GetParentProcessId(base::GetCurrentProcessHandle());
-  base::FilePath parent_image_path = GetProcessImagePath(parent_pid);
-
-  // On Windows, Chrome launches native messaging hosts via cmd for stdio
-  // communication. See:
-  //   chrome/browser/extensions/api/messaging/native_process_launcher_win.cc
-  // Therefore, we check if the parent is cmd and skip to the grandparent if
-  // that's the case. It's possible to do stdio communications without cmd, so
-  // we don't require the parent to always be cmd.
-
-  // Use IsSystemCmd() for comparison as a 64-bit Chrome launches a 64-bit
-  // cmd.exe which is in C:\Windows\System32, but for a 32-bit native messaging
-  // host, C:\Windows\System32 will be redirected to C:\Windows\SysWOW64, so we
-  // perform case-insensitive directory checks.
-  if (IsSystemCmd(parent_image_path)) {
-    // Skip to the grandparent.
-    base::win::ScopedHandle parent_handle(
-        OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, false, parent_pid));
-    if (parent_handle.is_valid()) {
-      parent_pid = base::GetParentProcessId(parent_handle.Get());
-      parent_image_path = GetProcessImagePath(parent_pid);
-    } else {
-      PLOG(ERROR) << "Failed to query parent info.";
-      return false;
-    }
+  base::ProcessId launcher_pid = GetLauncherProcessIdFromStdioPipes();
+  if (launcher_pid == base::kNullProcessId) {
+    LOG(ERROR) << "Failed to resolve launcher PID from stdio pipes.";
+    return false;
   }
 
-  return IsBinaryTrusted(parent_image_path);
+  base::FilePath launcher_image_path = GetProcessImagePath(launcher_pid);
+  if (launcher_image_path.empty()) {
+    LOG(ERROR) << "Failed to get launcher process image path.";
+    return false;
+  }
+
+  return IsBinaryTrusted(launcher_image_path);
 #elif BUILDFLAG(IS_MAC)
   // TODO: crbug.com/410903981 - move away from PID-based security checks, which
   // might be susceptible of PID reuse attacks, if Apple provides APIs to query
diff --git a/remoting/host/base/process_util_win_unittest.cc b/remoting/host/base/process_util_win_unittest.cc
new file mode 100644
index 0000000..2886447d
--- /dev/null
+++ b/remoting/host/base/process_util_win_unittest.cc
@@ -0,0 +1,104 @@
+// 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/base/process_util.h"
+
+#include <windows.h>
+
+#include "base/process/process.h"
+#include "base/process/process_handle.h"
+#include "base/win/scoped_handle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+
+namespace {
+
+class ScopedStdHandle {
+ public:
+  ScopedStdHandle(DWORD std_handle, HANDLE new_handle)
+      : std_handle_(std_handle), old_handle_(::GetStdHandle(std_handle)) {
+    ::SetStdHandle(std_handle_, new_handle);
+  }
+
+  ~ScopedStdHandle() { ::SetStdHandle(std_handle_, old_handle_); }
+
+ private:
+  DWORD std_handle_;
+  HANDLE old_handle_;
+};
+
+}  // namespace
+
+TEST(ProcessUtilWinTest, NonPipeStdinReturnsNull) {
+  base::win::ScopedHandle event(::CreateEvent(nullptr, TRUE, FALSE, nullptr));
+  ASSERT_TRUE(event.is_valid());
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(event.get(), event.get()),
+            base::kNullProcessId);
+
+  ScopedStdHandle scoped_stdin(STD_INPUT_HANDLE, event.get());
+  ASSERT_EQ(GetLauncherProcessIdFromStdioPipes(), base::kNullProcessId);
+}
+
+TEST(ProcessUtilWinTest, PipeServerPidResolution) {
+  HANDLE read_pipe = nullptr;
+  HANDLE write_pipe = nullptr;
+  SECURITY_ATTRIBUTES sa = {};
+  sa.nLength = sizeof(sa);
+  sa.bInheritHandle = TRUE;
+  ASSERT_TRUE(::CreatePipe(&read_pipe, &write_pipe, &sa, 0));
+
+  base::win::ScopedHandle scoped_read(read_pipe);
+  base::win::ScopedHandle scoped_write(write_pipe);
+
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(read_pipe, write_pipe),
+            base::GetCurrentProcId());
+
+  ScopedStdHandle scoped_stdin(STD_INPUT_HANDLE, read_pipe);
+  ScopedStdHandle scoped_stdout(STD_OUTPUT_HANDLE, write_pipe);
+  ASSERT_EQ(GetLauncherProcessIdFromStdioPipes(), base::GetCurrentProcId());
+}
+
+TEST(ProcessUtilWinTest, NullOrInvalidHandlesReturnNull) {
+  HANDLE read_pipe = nullptr;
+  HANDLE write_pipe = nullptr;
+  SECURITY_ATTRIBUTES sa = {};
+  sa.nLength = sizeof(sa);
+  sa.bInheritHandle = TRUE;
+  ASSERT_TRUE(::CreatePipe(&read_pipe, &write_pipe, &sa, 0));
+
+  base::win::ScopedHandle scoped_read(read_pipe);
+  base::win::ScopedHandle scoped_write(write_pipe);
+
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(nullptr, write_pipe),
+            base::kNullProcessId);
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(read_pipe, nullptr),
+            base::kNullProcessId);
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(INVALID_HANDLE_VALUE, write_pipe),
+            base::kNullProcessId);
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(read_pipe, INVALID_HANDLE_VALUE),
+            base::kNullProcessId);
+}
+
+TEST(ProcessUtilWinTest, MismatchedOrNonPipeHandlesReturnNull) {
+  HANDLE read_pipe = nullptr;
+  HANDLE write_pipe = nullptr;
+  SECURITY_ATTRIBUTES sa = {};
+  sa.nLength = sizeof(sa);
+  sa.bInheritHandle = TRUE;
+  ASSERT_TRUE(::CreatePipe(&read_pipe, &write_pipe, &sa, 0));
+
+  base::win::ScopedHandle scoped_read(read_pipe);
+  base::win::ScopedHandle scoped_write(write_pipe);
+
+  base::win::ScopedHandle event(::CreateEvent(nullptr, TRUE, FALSE, nullptr));
+  ASSERT_TRUE(event.is_valid());
+
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(read_pipe, event.get()),
+            base::kNullProcessId);
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(event.get(), write_pipe),
+            base::kNullProcessId);
+}
+
+}  // namespace remoting
diff --git a/remoting/BUILD.gn b/remoting/BUILD.gn
index 0cc19b5..99aca082 100644
--- a/remoting/BUILD.gn
+++ b/remoting/BUILD.gn
@@ -110,6 +110,7 @@
     deps += [
       "//remoting/codec:unit_tests",
       "//remoting/host:unit_tests",
+      "//remoting/host/base:unit_tests",
       "//ui/gfx",
     ]
     if (is_posix) {
diff --git a/remoting/host/base/BUILD.gn b/remoting/host/base/BUILD.gn
index e487d2d..f4d6a0b 100644
--- a/remoting/host/base/BUILD.gn
+++ b/remoting/host/base/BUILD.gn
@@ -54,4 +54,8 @@
     "//testing/gtest",
     "//third_party/webrtc_overrides:webrtc_component",
   ]
+
+  if (is_win) {
+    sources += [ "process_util_win_unittest.cc" ]
+  }
 }

diff --git a/remoting/host/base/process_util_win_unittest.cc b/remoting/host/base/process_util_win_unittest.cc
new file mode 100644
index 0000000..2886447d
--- /dev/null
+++ b/remoting/host/base/process_util_win_unittest.cc
@@ -0,0 +1,104 @@
+// 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/base/process_util.h"
+
+#include <windows.h>
+
+#include "base/process/process.h"
+#include "base/process/process_handle.h"
+#include "base/win/scoped_handle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+
+namespace {
+
+class ScopedStdHandle {
+ public:
+  ScopedStdHandle(DWORD std_handle, HANDLE new_handle)
+      : std_handle_(std_handle), old_handle_(::GetStdHandle(std_handle)) {
+    ::SetStdHandle(std_handle_, new_handle);
+  }
+
+  ~ScopedStdHandle() { ::SetStdHandle(std_handle_, old_handle_); }
+
+ private:
+  DWORD std_handle_;
+  HANDLE old_handle_;
+};
+
+}  // namespace
+
+TEST(ProcessUtilWinTest, NonPipeStdinReturnsNull) {
+  base::win::ScopedHandle event(::CreateEvent(nullptr, TRUE, FALSE, nullptr));
+  ASSERT_TRUE(event.is_valid());
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(event.get(), event.get()),
+            base::kNullProcessId);
+
+  ScopedStdHandle scoped_stdin(STD_INPUT_HANDLE, event.get());
+  ASSERT_EQ(GetLauncherProcessIdFromStdioPipes(), base::kNullProcessId);
+}
+
+TEST(ProcessUtilWinTest, PipeServerPidResolution) {
+  HANDLE read_pipe = nullptr;
+  HANDLE write_pipe = nullptr;
+  SECURITY_ATTRIBUTES sa = {};
+  sa.nLength = sizeof(sa);
+  sa.bInheritHandle = TRUE;
+  ASSERT_TRUE(::CreatePipe(&read_pipe, &write_pipe, &sa, 0));
+
+  base::win::ScopedHandle scoped_read(read_pipe);
+  base::win::ScopedHandle scoped_write(write_pipe);
+
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(read_pipe, write_pipe),
+            base::GetCurrentProcId());
+
+  ScopedStdHandle scoped_stdin(STD_INPUT_HANDLE, read_pipe);
+  ScopedStdHandle scoped_stdout(STD_OUTPUT_HANDLE, write_pipe);
+  ASSERT_EQ(GetLauncherProcessIdFromStdioPipes(), base::GetCurrentProcId());
+}
+
+TEST(ProcessUtilWinTest, NullOrInvalidHandlesReturnNull) {
+  HANDLE read_pipe = nullptr;
+  HANDLE write_pipe = nullptr;
+  SECURITY_ATTRIBUTES sa = {};
+  sa.nLength = sizeof(sa);
+  sa.bInheritHandle = TRUE;
+  ASSERT_TRUE(::CreatePipe(&read_pipe, &write_pipe, &sa, 0));
+
+  base::win::ScopedHandle scoped_read(read_pipe);
+  base::win::ScopedHandle scoped_write(write_pipe);
+
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(nullptr, write_pipe),
+            base::kNullProcessId);
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(read_pipe, nullptr),
+            base::kNullProcessId);
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(INVALID_HANDLE_VALUE, write_pipe),
+            base::kNullProcessId);
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(read_pipe, INVALID_HANDLE_VALUE),
+            base::kNullProcessId);
+}
+
+TEST(ProcessUtilWinTest, MismatchedOrNonPipeHandlesReturnNull) {
+  HANDLE read_pipe = nullptr;
+  HANDLE write_pipe = nullptr;
+  SECURITY_ATTRIBUTES sa = {};
+  sa.nLength = sizeof(sa);
+  sa.bInheritHandle = TRUE;
+  ASSERT_TRUE(::CreatePipe(&read_pipe, &write_pipe, &sa, 0));
+
+  base::win::ScopedHandle scoped_read(read_pipe);
+  base::win::ScopedHandle scoped_write(write_pipe);
+
+  base::win::ScopedHandle event(::CreateEvent(nullptr, TRUE, FALSE, nullptr));
+  ASSERT_TRUE(event.is_valid());
+
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(read_pipe, event.get()),
+            base::kNullProcessId);
+  ASSERT_EQ(GetLauncherProcessIdFromPipes(event.get(), write_pipe),
+            base::kNullProcessId);
+}
+
+}  // namespace remoting