From 02937399c63b486b3353dd607a10cd31cd56bc89 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Thu, 4 Jun 2026 09:54:51 -0700 Subject: [PATCH] fix(security): handle remaining from_utf8_lossy decision/scan sites (WI-4.3 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tightened anti-pattern gate surfaced 9 from_utf8_lossy sites beyond the 3 files WI-4.3 first covered. Strict-parse the one real decision (system_status parent-process-name → fail-closed); AUDIT-OK(bytes) the rest with precise reasons: daemon-identity substring probes that only fail-safe (connect_sync_autostart ×2, mcp_pid), per-line PID scans, HTTP display bodies, a locally-assembled buffer, and macOS memory-stats parses that already fail closed. Gate from_utf8_lossy flags: 9 → 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/uffs-cli/src/commands/system_status.rs | 10 +++++++++- crates/uffs-client/src/connect_sync_autostart.rs | 6 ++++++ crates/uffs-client/src/mcp_pid.rs | 2 ++ crates/uffs-mcp/src/handler/definitions.rs | 2 ++ crates/uffs-mft/src/platform/system.rs | 5 +++++ 5 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/uffs-cli/src/commands/system_status.rs b/crates/uffs-cli/src/commands/system_status.rs index 6419c9764..0e17470ff 100644 --- a/crates/uffs-cli/src/commands/system_status.rs +++ b/crates/uffs-cli/src/commands/system_status.rs @@ -384,6 +384,9 @@ fn find_mcp_stdio_processes() -> Vec { else { return Vec::new(); }; + // AUDIT-OK(bytes): per-line PID scan of process-list output; each line's + // PID parses-or-skips (fail-safe). Whole-buffer strict decode would drop + // the whole list on one bad byte. (WI-4.3 follow-up) let text = String::from_utf8_lossy(&raw_output.stdout); let my_pid = std::process::id(); @@ -475,7 +478,10 @@ fn resolve_parent_name(ppid: u32) -> Option { .stderr(std::process::Stdio::null()) .output() .ok()?; - let name = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + // Strict decode: this process name is returned and used for a + // comparison/targeting decision, so invalid UTF-8 fails closed (None) + // rather than yielding a U+FFFD-mangled name. (WI-4.3 follow-up) + let name = core::str::from_utf8(&output.stdout).ok()?.trim().to_owned(); if name.is_empty() { return None; } @@ -504,6 +510,8 @@ fn http_get_json(bind: &str, port: u16, path: &str) -> Result let mut response = Vec::new(); stream.read_to_end(&mut response)?; + // AUDIT-OK(bytes): HTTP probe response body split for display only, not a + // trust/targeting decision. (WI-4.3 follow-up) let text = String::from_utf8_lossy(&response); let body = text .split_once("\r\n\r\n") diff --git a/crates/uffs-client/src/connect_sync_autostart.rs b/crates/uffs-client/src/connect_sync_autostart.rs index db6f1af75..114714110 100644 --- a/crates/uffs-client/src/connect_sync_autostart.rs +++ b/crates/uffs-client/src/connect_sync_autostart.rs @@ -86,6 +86,9 @@ fn is_process_alive(pid: u32) -> bool { .stderr(std::process::Stdio::null()) .output() .is_ok_and(|output| { + // AUDIT-OK(bytes): daemon-identity probe via substring match; a lossy + // decode can only FAIL the match → treat as 'not the daemon' (fail-safe + // reconnect), never a false positive. (WI-4.3 follow-up) let text = String::from_utf8_lossy(&output.stdout); // tasklist prints "uffsd.exe ..." when the process matches. // Verify both the PID and the executable name. @@ -113,6 +116,9 @@ fn is_daemon_process(pid: u32) -> bool { .stderr(std::process::Stdio::null()) .output() .is_ok_and(|output| { + // AUDIT-OK(bytes): daemon-identity probe via substring match; a lossy + // decode can only FAIL the match → treat as 'not the daemon' (fail-safe + // reconnect), never a false positive. (WI-4.3 follow-up) let comm = String::from_utf8_lossy(&output.stdout); // `ps -o comm=` prints the executable path or basename. // Match if any path component is "uffsd". diff --git a/crates/uffs-client/src/mcp_pid.rs b/crates/uffs-client/src/mcp_pid.rs index 0dd2f2ff0..e54d33f0c 100644 --- a/crates/uffs-client/src/mcp_pid.rs +++ b/crates/uffs-client/src/mcp_pid.rs @@ -200,6 +200,8 @@ fn is_process_alive(pid: u32) -> bool { std::process::Command::new("tasklist") .args(["/FI", &format!("PID eq {pid}"), "/NH"]) .output() + // AUDIT-OK(bytes): tasklist substring PID check; lossy can only fail the + // match (fail-safe). (WI-4.3 follow-up) .is_ok_and(|output| String::from_utf8_lossy(&output.stdout).contains(&pid.to_string())) } } diff --git a/crates/uffs-mcp/src/handler/definitions.rs b/crates/uffs-mcp/src/handler/definitions.rs index 976862e8c..fe0a7d792 100644 --- a/crates/uffs-mcp/src/handler/definitions.rs +++ b/crates/uffs-mcp/src/handler/definitions.rs @@ -284,5 +284,7 @@ pub(crate) fn percent_decode_path(encoded: &str) -> String { } idx += 1; } + // AUDIT-OK(bytes): final decode of a locally-assembled buffer for display. + // (WI-4.3 follow-up) String::from_utf8_lossy(&decoded).into_owned() } diff --git a/crates/uffs-mft/src/platform/system.rs b/crates/uffs-mft/src/platform/system.rs index 9a1ca9fba..f921c6a18 100644 --- a/crates/uffs-mft/src/platform/system.rs +++ b/crates/uffs-mft/src/platform/system.rs @@ -734,11 +734,16 @@ fn query_memory_macos() -> Option { .arg("hw.memsize") .output() .ok()?; + // AUDIT-OK(bytes): sysctl memsize for a stats display; the following + // .parse().ok()? already fails closed on any non-numeric/garbage byte. + // (WI-4.3 follow-up) let total_str = String::from_utf8_lossy(&total_out.stdout); let total_bytes: u64 = total_str.trim().parse().ok()?; // Available: vm_stat → parse "Pages free" and "Pages inactive" let vm_out = Command::new("vm_stat").output().ok()?; + // AUDIT-OK(bytes): vm_stat output parsed line-by-line for a stats display; + // each field parse fails closed. (WI-4.3 follow-up) let vm_str = String::from_utf8_lossy(&vm_out.stdout); // First line: "Mach Virtual Memory Statistics: (page size of 16384 bytes)"