🛡️ Sentinel: Fix Windows cmd.exe environment variable injection

src/modules/command.rs

@@ -12,7 +12,7 @@ use super::{
     ParamExt,
 };
 use crate::connection::{Connection, ExecuteOptions};
-use crate::utils::{cmd_escape, powershell_escape, shell_escape};
+use crate::utils::{cmd_arg_escape, powershell_escape, shell_escape};
 use once_cell::sync::Lazy;
 use std::path::Path;
 use std::process::Command;
@@ -89,7 +89,7 @@ impl CommandModule {
             let escaped_args: Vec<std::borrow::Cow<'_, str>> = argv
                 .iter()
                 .map(|arg| match shell_type.as_str() {
-                    "cmd" => cmd_escape(arg),
+                    "cmd" => cmd_arg_escape(arg),
                     "powershell" => powershell_escape(arg),
                     "posix" | "sh" | "bash" => shell_escape(arg),
                     _ => shell_escape(arg), // Default to POSIX for safety/backward compatibility

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 {
@@ -429,6 +429,7 @@ pub fn validate_command_args(args: &str) -> ModuleResult<()> {
         ("!", "history expansion !"),
         ("\\", "shell escaping \\"),
         ("$", "variable expansion $"),
+        ("%", "variable expansion %"),
         ("#", "shell comment #"),
     ];
 
@@ -2080,6 +2081,7 @@ mod tests {
         // Extended checks
         assert!(validate_command_args("bash;echo").is_err());
         assert!(validate_command_args("cmd&").is_err());
+        assert!(validate_command_args("echo %USERNAME%").is_err());
     }
 
     #[test]

src/utils/mod.rs

@@ -187,6 +187,39 @@ pub fn cmd_escape(s: &str) -> Cow<'_, str> {
     Cow::Owned(escaped)
 }
 
+/// Escape a string for use as a single argument in Windows cmd.exe.
+///
+/// This function wraps the string in double quotes and escapes any internal double quotes,
+/// similar to `cmd_escape`. Additionally, it escapes `%` characters by replacing them
+/// with `%""` to prevent variable expansion inside the quoted string.
+///
+/// This is crucial because `cmd.exe` performs variable expansion even inside double quotes.
+/// By inserting an empty string `""` after the `%`, we break the variable name token
+/// without changing the value (since `""` is empty), effectively preventing expansion.
+///
+/// # Arguments
+///
+/// * `s` - The argument string to escape
+///
+/// # Returns
+///
+/// * The escaped argument string safe for cmd.exe execution as a literal
+pub fn cmd_arg_escape(s: &str) -> Cow<'_, str> {
+    let mut escaped = String::with_capacity(s.len() + 16);
+    escaped.push('"');
+
+    for c in s.chars() {
+        match c {
+            '"' => escaped.push_str("\"\""),
+            '%' => escaped.push_str("%\"\""),
+            _ => escaped.push(c),
+        }
+    }
+
+    escaped.push('"');
+    Cow::Owned(escaped)
+}
+
 /// Escapes a string for safe use in PowerShell commands.
 ///
 /// This function handles special characters that could cause issues
@@ -321,6 +354,17 @@ mod tests {
         assert_eq!(cmd_escape(""), "\"\"");
     }
 
+    #[test]
+    fn test_cmd_arg_escape() {
+        assert_eq!(cmd_arg_escape("simple"), "\"simple\"");
+        assert_eq!(cmd_arg_escape("with space"), "\"with space\"");
+        assert_eq!(cmd_arg_escape("with\"quote"), "\"with\"\"quote\"");
+        // Verify % is escaped with %""
+        assert_eq!(cmd_arg_escape("%USERNAME%"), "\"%\"\"USERNAME%\"\"\"");
+        assert_eq!(cmd_arg_escape("100%"), "\"100%\"\"\"");
+        assert_eq!(cmd_arg_escape(""), "\"\"");
+    }
+
     #[test]
     fn test_powershell_escape() {
         assert_eq!(powershell_escape("simple"), "'simple'");

tests/security_windows_command_injection.rs

@@ -0,0 +1,43 @@
+use rustible::modules::{validate_command_args, command::CommandModule, Module, ModuleParams, ModuleContext};
+use rustible::utils::cmd_arg_escape;
+use std::collections::HashMap;
+
+#[test]
+fn test_validate_command_args_blocks_percent() {
+    // This should now fail (Err) because we added % to dangerous_patterns
+    assert!(validate_command_args("echo %USERNAME%").is_err());
+}
+
+#[test]
+fn test_cmd_arg_escape_escapes_percent() {
+    let input = "%USERNAME%";
+    let escaped = cmd_arg_escape(input);
+    // Should now return "%""USERNAME%" inside quotes (outer quotes + inner escaped quotes)
+    // cmd_arg_escape wraps in "...", so result is "%""USERNAME%"
+    // Wait, escaped string content: "%""USERNAME%"
+    // Representation in Rust string literal: "\"%\"\"USERNAME%\"\"\""
+    assert_eq!(escaped, "\"%\"\"USERNAME%\"\"\"");
+}
+
+#[test]
+fn test_command_module_argv_escapes_percent() {
+    let module = CommandModule;
+    let mut params: ModuleParams = HashMap::new();
+    params.insert(
+        "argv".to_string(),
+        serde_json::json!(["echo", "%USERNAME%"]),
+    );
+    params.insert("shell_type".to_string(), serde_json::json!("cmd"));
+
+    let context = ModuleContext::default().with_check_mode(true);
+
+    let result = module.execute(&params, &context).unwrap();
+    let msg = result.msg;
+
+    // msg format: "Would execute: <cmd>"
+    // cmd should be: "echo" "%""USERNAME%"
+    // Expected substring in msg: "\"%\"\"USERNAME%\"\"\""
+
+    println!("Message: {}", msg);
+    assert!(msg.contains("\"%\"\"USERNAME%\"\"\""));
+}