From 8c8cb49c237eec2f82192dd831ec7308905f2c0a Mon Sep 17 00:00:00 2001 From: JJ Liebig Date: Mon, 28 Sep 2026 13:16:10 +0200 Subject: [PATCH] fix(windows): read agent command lines without PROCESS_VM_READ (#4708) * fix(windows): read agent command lines without PROCESS_VM_READ Windows process command lines were read by opening each process with PROCESS_VM_READ and walking its PEB. Hardened runtimes (Electron/Node) and security products deny that access, so those processes resolved to a bare `node.exe`/`bun.exe` name with no argv, argv0, or cmdline. A runtime-wrapped agent launched from such a process was then indistinguishable from an unrelated runtime process, and the pane stayed on the shell with `agent_status: unknown`. Prefer `NtQueryInformationProcess` with `ProcessCommandLineInformation` (Windows 8.1+), which needs only `PROCESS_QUERY_LIMITED_INFORMATION`, and keep the existing PEB read as a fallback for processes that do not expose a stored command line. The fallback still requires `PROCESS_VM_READ`, so this only widens coverage. refs #4579 * fix(windows): handle growing command lines before failure test The retry added for a command line that grows between the size probe and the read was unreachable. NTSTATUS is signed, so STATUS_BUFFER_OVERFLOW, STATUS_BUFFER_TOO_SMALL, and STATUS_INFO_LENGTH_MISMATCH are all negative and were treated as failures by the earlier `status < 0` check. Check the growth statuses first and retry once with the reported size. Also read the returned UNICODE_STRING header unaligned, since a Vec buffer only guarantees byte alignment. * test(windows): keep command-line marker test process alive Passing the marker as a second argument to `ping` let `ping` exit as soon as it saw one target, so the command-line query could race the process exit. Move the marker into a `rem` after `&` so it stays in cmd.exe's own command line and `ping -n 11` keeps the process alive for the read. --- src/detect/mod.rs | 3 + src/platform/windows.rs | 153 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 155 insertions(+), 1 deletion(-) diff --git a/src/detect/mod.rs b/src/detect/mod.rs index 146c30894..37d5481a8 100644 --- a/src/detect/mod.rs +++ b/src/detect/mod.rs @@ -1497,6 +1497,9 @@ mod tests { #[test] fn identify_agent_in_job_detects_node_wrapped_pi_bundled_cli() { + // Hardened runtimes deny `PROCESS_VM_READ`, so the command line can come + // from a LimitedInformation query with the launcher path intact. The + // detection path must not depend on how that command line was obtained. let job = crate::platform::ForegroundJob { process_group_id: 123, processes: vec![foreground_process( diff --git a/src/platform/windows.rs b/src/platform/windows.rs index d96570a63..292be4a44 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -315,11 +315,13 @@ pub(crate) fn set_default_plugin_pane_pwd( } use windows_sys::{ + Wdk::System::Threading::ProcessCommandLineInformation, Wdk::System::Threading::{NtQueryInformationProcess, ProcessBasicInformation}, Win32::{ Foundation::{ CloseHandle, GlobalFree, LocalFree, FILETIME, HANDLE, HWND, INVALID_HANDLE_VALUE, - MAX_PATH, NTSTATUS, STATUS_SUCCESS, UNICODE_STRING, + MAX_PATH, NTSTATUS, STATUS_BUFFER_OVERFLOW, STATUS_BUFFER_TOO_SMALL, + STATUS_INFO_LENGTH_MISMATCH, STATUS_SUCCESS, UNICODE_STRING, }, Globalization::{CompareStringOrdinal, CSTR_EQUAL, CSTR_GREATER_THAN, CSTR_LESS_THAN}, Security::SECURITY_ATTRIBUTES, @@ -1829,6 +1831,19 @@ impl ProcessSnapshotCache { } fn read_process_command(pid: u32, name: &str) -> WindowsProcessCommand { + // Prefer the command-line information class: it needs only + // `PROCESS_QUERY_LIMITED_INFORMATION`, while the PEB path below also needs + // `PROCESS_VM_READ`, which hardened runtimes (Electron/Node) and security + // products deny. Without a command line an agent launched through a runtime + // is indistinguishable from a bare `node.exe`/`bun.exe` process, so the + // pane would never register as an agent. + if let Some(process) = ProcessHandle::open(pid, PROCESS_QUERY_LIMITED_INFORMATION) { + if let Some(cmdline) = read_process_command_line(process.0) { + let creation_time = process_creation_time(process.0); + return WindowsProcessCommand::from_cmdline(name, creation_time, Some(cmdline)); + } + } + let Some(process) = ProcessHandle::open(pid, PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_VM_READ) else { @@ -2140,6 +2155,89 @@ fn environment_variable_from_utf16(environment: &[u16], name: &str) -> Option Option { + let mut required = 0_u32; + // SAFETY: a null buffer with length 0 only asks for the required size, and + // `required` is a valid out-pointer for the duration of the call. + let status = unsafe { + NtQueryInformationProcess( + process, + ProcessCommandLineInformation, + null_mut(), + 0, + &mut required, + ) + }; + // A process with no command line is already handled as a miss below. + if status != STATUS_BUFFER_TOO_SMALL + && status != STATUS_INFO_LENGTH_MISMATCH + && status != STATUS_BUFFER_OVERFLOW + { + return None; + } + + // `required` is already at least a UNICODE_STRING sized buffer. + let mut buffer = vec![0_u8; required as usize]; + for _ in 0..2 { + // SAFETY: `buffer` is `required` bytes and both pointers are valid for + // the call; the kernel writes the length back into `required`. + let status = unsafe { + NtQueryInformationProcess( + process, + ProcessCommandLineInformation, + buffer.as_mut_ptr().cast(), + required, + &mut required, + ) + }; + // These three statuses all mean the command line grew between the probe + // and the read. Some data was written; retry once with the larger buffer + // the call just reported. They are negative as `NTSTATUS`, so they must + // be checked before the failure test below. + let grew = status == STATUS_BUFFER_OVERFLOW + || status == STATUS_BUFFER_TOO_SMALL + || status == STATUS_INFO_LENGTH_MISMATCH; + if grew { + buffer = vec![0_u8; required as usize]; + continue; + } + if status < 0 { + return None; + } + break; + } + + // SAFETY: on success the kernel wrote a UNICODE_STRING followed by its + // UTF-16 contents into `buffer`. A `Vec` only guarantees byte + // alignment, so read the header unaligned. + let unicode = unsafe { buffer.as_ptr().cast::().read_unaligned() }; + let length = usize::from(unicode.Length); + // A short command line leaves `Length` inside the header itself; guard + // against reading a malformed header as string data. + if length == 0 || !length.is_multiple_of(2) { + return None; + } + let string_offset = size_of::(); + if string_offset + length > buffer.len() { + return None; + } + let units = buffer[string_offset..string_offset + length] + .chunks_exact(2) + .map(|unit| u16::from_ne_bytes([unit[0], unit[1]])) + .collect::>(); + String::from_utf16(&units) + .ok() + .filter(|command_line| !command_line.is_empty()) +} + fn read_process_parameters(process: HANDLE) -> Option { let mut basic_info = MaybeUninit::::uninit(); let status = unsafe { @@ -3612,6 +3710,59 @@ mod tests { assert_eq!(observed.as_deref(), Some("pane-test")); } + #[test] + fn windows_process_command_line_reads_live_process_with_limited_access() { + // The point of the fix: the command line must be readable from a handle + // that does not request `PROCESS_VM_READ`. Verification against a + // process that actually denies that access needs a hardened host, which + // this suite cannot provide. + let handle = super::ProcessHandle::open( + std::process::id(), + super::PROCESS_QUERY_LIMITED_INFORMATION, + ) + .expect("open self with limited access"); + + let command_line = + super::read_process_command_line(handle.0).expect("command line must be readable"); + assert!( + !command_line.is_empty(), + "command line for the test process must not be empty" + ); + } + + #[test] + fn windows_process_command_line_reads_spawned_process_marker() { + let shell = + std::env::var_os("ComSpec").unwrap_or_else(|| r"C:\Windows\System32\cmd.exe".into()); + // `rem` keeps the marker inside cmd.exe's own command line without + // becoming a target for `ping`, so the process stays alive for the read. + let mut child = Command::new(shell) + .args([ + "/D", + "/Q", + "/C", + "ping -n 11 127.0.0.1 > NUL & rem unique-cmdline-marker", + ]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("spawn cmd"); + + let command_line = + super::ProcessHandle::open(child.id(), super::PROCESS_QUERY_LIMITED_INFORMATION) + .and_then(|process| super::read_process_command_line(process.0)); + + let _ = child.kill(); + let _ = child.wait(); + + let command_line = command_line.expect("command line must be readable"); + assert!( + command_line.contains("unique-cmdline-marker"), + "unexpected command line: {command_line}" + ); + } + #[test] fn windows_process_tree_selects_direct_agent_descendant() { let entries = vec![