keyhog_core/
safe_bin.rs

1//! Safe absolute-path resolution for external binaries we shell out to.
2//!
3//! Defends against `PATH` injection (kimi-wave1 audit finding 3.PATH-x):
4//! `Command::new("git")` lets the user's `PATH` decide which `git` we
5//! actually invoke. An attacker who can prepend a directory to `PATH` —
6//! a CI runner stage, a malicious dotfile, an override in
7//! `~/.config/fish/config.fish` — substitutes their own binary. Since
8//! keyhog feeds the binary credential bytes (via env vars / argv / stdin
9//! during git scans), that's a credential-exfil pivot.
10//!
11//! This module enumerates a hardcoded allowlist of system binary
12//! directories and returns the FIRST match. Anything not in those dirs
13//! is refused. The allowlist is intentionally narrow — distro-shipped
14//! binaries only. If your environment legitimately needs a different
15//! path, set the `KEYHOG_TRUSTED_BIN_DIR` env var (colon-separated on
16//! Unix, semicolon-separated on Windows) — but be aware that anyone
17//! who can set that env var can already inject anyway, so the env-var
18//! path exists for ops convenience, not as a security boundary.
19
20use std::path::PathBuf;
21
22#[cfg(unix)]
23const SYSTEM_BIN_DIRS: &[&str] = &[
24    "/usr/bin",
25    "/usr/local/bin",
26    "/usr/local/sbin",
27    "/usr/sbin",
28    "/bin",
29    "/sbin",
30    "/opt/homebrew/bin", // macOS Apple Silicon
31    "/opt/homebrew/sbin",
32];
33
34#[cfg(windows)]
35const SYSTEM_BIN_DIRS: &[&str] = &[
36    "C:\\Windows\\System32",
37    "C:\\Windows",
38    "C:\\Windows\\System32\\WindowsPowerShell\\v1.0",
39    "C:\\Program Files\\Git\\cmd",
40    "C:\\Program Files\\Git\\bin",
41];
42
43#[cfg(unix)]
44const EXE_SUFFIXES: &[&str] = &[""];
45
46#[cfg(windows)]
47const EXE_SUFFIXES: &[&str] = &[".exe", ".com", ".bat", ".cmd"];
48
49/// Resolve `name` to an absolute path inside one of the trusted system
50/// binary directories. Returns `None` if not found in any trusted dir
51/// (do NOT fall back to `Command::new(name)` — that's exactly the bug).
52/// Resolve a binary name to an absolute path, defending against PATH injection.
53pub fn resolve_safe_bin(name: &str) -> Option<PathBuf> {
54    if name.contains('/') || name.contains('\\') {
55        // Caller already passed a path; only accept if it's absolute and
56        // points inside a trusted dir.
57        let p = PathBuf::from(name);
58        if p.is_absolute() && in_trusted_dir(&p) && p.exists() {
59            return Some(p);
60        }
61        return None;
62    }
63
64    let mut search_dirs: Vec<PathBuf> = SYSTEM_BIN_DIRS.iter().map(PathBuf::from).collect();
65    if let Ok(extra) = std::env::var("KEYHOG_TRUSTED_BIN_DIR") {
66        let sep = if cfg!(windows) { ';' } else { ':' };
67        for dir in extra.split(sep).filter(|s| !s.is_empty()) {
68            search_dirs.push(PathBuf::from(dir));
69        }
70    }
71
72    for dir in &search_dirs {
73        for suffix in EXE_SUFFIXES {
74            let candidate = dir.join(format!("{name}{suffix}"));
75            if candidate.is_file() {
76                return Some(candidate);
77            }
78        }
79    }
80    None
81}
82
83/// Resolve `name` or fall back to `Command::new(name)` semantics if
84/// nothing trusted was found. Intended for read-only / probe sites
85/// (hardware detection, version queries) where blocking the command
86/// would degrade UX more than the marginal risk warrants. Logs a
87/// warning so the operator knows the unsafe fallback fired.
88///
89/// For sites that handle credential bytes (git scans, docker pulls),
90/// use `resolve_safe_bin` directly and refuse on `None`.
91pub fn resolve_or_fallback(name: &str) -> PathBuf {
92    if let Some(p) = resolve_safe_bin(name) {
93        return p;
94    }
95    tracing::warn!(
96        "keyhog: '{name}' not found in trusted system bin dirs; falling back to PATH lookup. \
97         Set KEYHOG_TRUSTED_BIN_DIR if running on a non-standard distro."
98    );
99    PathBuf::from(name)
100}
101
102fn in_trusted_dir(p: &std::path::Path) -> bool {
103    let parent = match p.parent() {
104        Some(p) => p,
105        None => return false,
106    };
107    SYSTEM_BIN_DIRS
108        .iter()
109        .any(|d| parent == std::path::Path::new(d))
110}
111
112#[cfg(test)]
113mod tests {
114    use super::*;
115
116    #[test]
117    #[cfg(unix)]
118    fn resolves_sh_to_known_path() {
119        // `/bin/sh` exists on every Unix variant we ship to.
120        let resolved = resolve_safe_bin("sh").expect("sh should resolve");
121        assert!(resolved.is_absolute());
122        assert!(resolved.ends_with("sh"));
123    }
124
125    #[test]
126    fn refuses_relative_path() {
127        assert!(resolve_safe_bin("./malicious").is_none());
128        assert!(resolve_safe_bin("../../../bin/sh").is_none());
129    }
130
131    #[test]
132    fn refuses_absolute_path_outside_trusted_dirs() {
133        assert!(resolve_safe_bin("/tmp/whatever").is_none());
134    }
135
136    #[test]
137    fn unknown_binary_is_none() {
138        // A name that should never exist on any system.
139        assert!(resolve_safe_bin("definitely-not-a-real-binary-xyz123").is_none());
140    }
141}
keyhog_core/safe_bin.rs

keyhog_core/
safe_bin.rs
