From a70204ab8c09d6a1cdd10dec54d1670045e153e0 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 23 Feb 2026 11:25:38 +0000 Subject: [PATCH] feat: Harden Windows command execution against injection - Enhance `cmd_escape` in `src/utils/mod.rs` to escape `%` characters as `%""` to prevent environment variable expansion in `cmd.exe`. - Update `validate_command_args` in `src/modules/mod.rs` to treat `%` and `^` as dangerous patterns, preventing their use in raw command strings where they could facilitate injection or information disclosure. - Add unit tests for both changes to verify security hardening. This change mitigates potential command injection and unintended variable expansion vulnerabilities on Windows systems. Co-authored-by: dolagoartur <146357947+dolagoartur@users.noreply.github.com> --- src/modules/mod.rs | 13 +++++++++++-- src/utils/mod.rs | 9 +++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/modules/mod.rs b/src/modules/mod.rs index b73cf0bd..09030475 100644 --- a/src/modules/mod.rs +++ b/src/modules/mod.rs @@ -393,11 +393,11 @@ pub fn validate_command_args(args: &str) -> ModuleResult<()> { // If the string contains only safe characters, we can skip the detailed check. // This avoids checking 24 patterns for every safe string (O(N) vs O(M*N)). // - // Safe characters: alphanumeric, space, _, -, ., /, :, +, =, ,, @, % + // Safe characters: alphanumeric, space, _, -, ., /, :, +, =, ,, @ let is_safe = args.bytes().all(|b| matches!(b, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b' ' | b'_' | b'-' | b'.' | b'/' | b':' | - b'+' | b'=' | b',' | b'@' | b'%' + b'+' | b'=' | b',' | b'@' )); if is_safe { @@ -430,6 +430,8 @@ pub fn validate_command_args(args: &str) -> ModuleResult<()> { ("\\", "shell escaping \\"), ("$", "variable expansion $"), ("#", "shell comment #"), + ("%", "variable expansion %"), + ("^", "cmd.exe escape ^"), ]; for (pattern, description) in dangerous_patterns { @@ -2090,6 +2092,13 @@ mod tests { assert!(validate_command_args("bash -c 'echo pwned' #").is_err()); } + #[test] + fn test_validate_command_args_rejects_windows_metachars() { + assert!(validate_command_args("echo %USERNAME%").is_err()); + assert!(validate_command_args("echo ^& calc.exe").is_err()); + assert!(validate_command_args("%COMSPEC%").is_err()); + } + #[test] fn test_get_remote_tmp() { // Test default diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 5fa508e7..d10645fb 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -163,6 +163,10 @@ pub fn cmd_escape(s: &str) -> Cow<'_, str> { for c in s.chars() { if c == '"' { escaped.push_str("\"\""); + } else if c == '%' { + // Insert empty string ("") after % to prevent variable expansion in cmd.exe + // e.g. %VAR% becomes %""VAR%"" + escaped.push_str("%\"\""); } else { escaped.push(c); } @@ -319,6 +323,11 @@ mod tests { assert_eq!(cmd_escape("foo&bar"), "\"foo&bar\""); assert_eq!(cmd_escape("foo|bar"), "\"foo|bar\""); assert_eq!(cmd_escape(""), "\"\""); + + // Test environment variable expansion prevention + // % is replaced with %"" which breaks the variable token in cmd.exe + assert_eq!(cmd_escape("%USERNAME%"), "\"%\"\"USERNAME%\"\"\""); + assert_eq!(cmd_escape("foo%bar"), "\"foo%\"\"bar\""); } #[test]