diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 1a9528255..8eb42ab72 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -17,3 +17,8 @@ **Vulnerability:** The `verify_github_sig` function in `apps/server/src/infra_routes.rs` verified GitHub webhook signatures using standard string comparison `expected.eq_ignore_ascii_case(hex_sig)`. **Learning:** String comparison functions exit early as soon as a character mismatch is found. In a cryptographic context, this variable response time leaks information and allows an attacker to perform a timing attack to forge a valid HMAC signature by guessing it byte by byte. **Prevention:** Always use a constant-time comparison library (like `hmac`'s `verify_slice()`) to compare cryptographic hashes, secrets, and signatures to prevent side-channel timing attacks. Avoid custom implementations of standard cryptographic algorithms. + +## 2024-05-24 - Workdir Command Validation Vulnerability +**Vulnerability:** The command validator (`validate_command` in `workdir_cmd.rs`) used `joined.to_ascii_lowercase().contains(bad)` against a list of blocked tokens like `rm ` to prevent command injection and destructive behavior. This is vulnerable if an attacker provides a command like `npm run rm`, which bypasses `rm ` (because there's no trailing space) but might still execute `rm`. Furthermore, the application relies solely on string matching, creating a bypass risk. +**Learning:** Checking for substrings against joined shell commands doesn't sufficiently protect against commands without expected spacing or specific subcommands. When validating inputs for a process executor, we need to inspect both the full joined string (for multi-word blocked phrases) AND the individual tokenized arguments to ensure exact matches for sensitive/blocked binaries or commands are caught. Relying solely on one method breaks either multi-word substring blocks or exact single word matches. +**Prevention:** Iteratively check each tokenized argument exactly against dangerous terms like `"rm"`, `"chmod"`, or `"chown"` in addition to substring checks on the entire command string. diff --git a/crates/agent/src/tools/workdir_cmd.rs b/crates/agent/src/tools/workdir_cmd.rs index 46a5ff54a..04e6c9edc 100644 --- a/crates/agent/src/tools/workdir_cmd.rs +++ b/crates/agent/src/tools/workdir_cmd.rs @@ -112,6 +112,13 @@ fn validate_command(raw: &str) -> std::result::Result, String> { return Err(format!("token interdit dans la commande : {bad}")); } } + + for p in &parts { + let p_lower = p.to_ascii_lowercase(); + if p_lower == "rm" || p_lower == "chmod" || p_lower == "chown" { + return Err(format!("token interdit dans la commande : {p}")); + } + } let bin = parts[0].as_str(); let bin_base = Path::new(bin) .file_name() @@ -340,6 +347,13 @@ mod tests { assert_eq!(p[0], "npm"); } + #[test] + fn blocks_npm_run_rm() { + assert!(validate_command("npm run rm").is_err()); + assert!(validate_command("npm run chmod").is_err()); + assert!(validate_command("npm run chown").is_err()); + } + #[test] fn blocks_pipe() { assert!(validate_command("npm run build | cat").is_err());