Skip to content

Commit 3ebcd39

Browse files
committed
fix(execpolicy): normalize commands to prevent validation bypass
Fixes bypass attempts where blocked commands like 'rm -rf' could be evaded using: - Extra whitespace: 'rm -rf' → 'rm -rf' - Quoted parts: "'rm' -rf" → 'rm -rf' - Path variants: '/bin/rm -rf' → 'rm -rf' Added normalize_command() function that: 1. Collapses whitespace by splitting/joining 2. Strips surrounding quotes from command parts 3. Extracts basename for command (first part) Also added comprehensive tests for bypass scenarios.
1 parent 829f773 commit 3ebcd39

1 file changed

Lines changed: 102 additions & 6 deletions

File tree

src/cortex-engine/src/validation.rs

Lines changed: 102 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,34 @@ pub struct CommandValidator {
269269
pub allow_shell_operators: bool,
270270
}
271271

272+
/// Normalize a command string for consistent validation.
273+
///
274+
/// This function handles bypass attempts such as:
275+
/// - Extra whitespace: "rm -rf" → "rm -rf"
276+
/// - Quoted parts: "'rm' -rf" → "rm -rf"
277+
/// - Path variants: "/bin/rm -rf" → "rm -rf"
278+
fn normalize_command(cmd: &str) -> String {
279+
cmd.split_whitespace()
280+
.enumerate()
281+
.map(|(idx, part)| {
282+
// Remove surrounding quotes (single and double)
283+
let unquoted = part
284+
.trim_matches(|c| c == '\'' || c == '"');
285+
286+
// For the first part (command), extract basename to handle path variants
287+
if idx == 0 {
288+
Path::new(unquoted)
289+
.file_name()
290+
.and_then(|name| name.to_str())
291+
.unwrap_or(unquoted)
292+
} else {
293+
unquoted
294+
}
295+
})
296+
.collect::<Vec<_>>()
297+
.join(" ")
298+
}
299+
272300
impl CommandValidator {
273301
/// Create a new validator.
274302
pub fn new() -> Self {
@@ -332,9 +360,12 @@ impl CommandValidator {
332360
));
333361
}
334362

335-
// Check allowed list
363+
// Normalize the command for consistent validation
364+
let normalized = normalize_command(command);
365+
366+
// Check allowed list using normalized command
336367
if let Some(ref allowed) = self.allowed {
337-
let cmd = command.split_whitespace().next().unwrap_or("");
368+
let cmd = normalized.split_whitespace().next().unwrap_or("");
338369
if !allowed.contains(cmd) {
339370
result.add_error(ValidationError::new(
340371
"command",
@@ -343,9 +374,10 @@ impl CommandValidator {
343374
}
344375
}
345376

346-
// Check blocked commands
377+
// Check blocked commands against normalized form
347378
for blocked in &self.blocked {
348-
if command.contains(blocked) {
379+
let normalized_blocked = normalize_command(blocked);
380+
if normalized.contains(&normalized_blocked) {
349381
result.add_error(ValidationError::new(
350382
"command",
351383
"Command contains blocked pattern",
@@ -354,9 +386,9 @@ impl CommandValidator {
354386
}
355387
}
356388

357-
// Check blocked patterns
389+
// Check blocked patterns against both original and normalized
358390
for pattern in &self.blocked_patterns {
359-
if command.contains(pattern) {
391+
if command.contains(pattern) || normalized.contains(pattern) {
360392
result.add_error(ValidationError::new(
361393
"command",
362394
"Command contains dangerous pattern",
@@ -700,6 +732,70 @@ mod tests {
700732
assert!(result.valid);
701733
}
702734

735+
#[test]
736+
fn test_command_validation_whitespace_bypass() {
737+
let validator = CommandValidator::new();
738+
739+
// Extra whitespace should not bypass validation
740+
let result = validator.validate("rm -rf /");
741+
assert!(!result.valid, "Extra whitespace should not bypass blocked command");
742+
743+
let result = validator.validate("rm -rf /");
744+
assert!(!result.valid, "Multiple spaces should not bypass blocked command");
745+
}
746+
747+
#[test]
748+
fn test_command_validation_quote_bypass() {
749+
let validator = CommandValidator::new();
750+
751+
// Quoted commands should not bypass validation
752+
let result = validator.validate("'rm' -rf /");
753+
assert!(!result.valid, "Single quotes should not bypass blocked command");
754+
755+
let result = validator.validate("\"rm\" -rf /");
756+
assert!(!result.valid, "Double quotes should not bypass blocked command");
757+
758+
let result = validator.validate("'rm' '-rf' '/'");
759+
assert!(!result.valid, "Fully quoted command should not bypass blocked command");
760+
}
761+
762+
#[test]
763+
fn test_command_validation_path_bypass() {
764+
let validator = CommandValidator::new();
765+
766+
// Path variants should not bypass validation
767+
let result = validator.validate("/bin/rm -rf /");
768+
assert!(!result.valid, "Absolute path should not bypass blocked command");
769+
770+
let result = validator.validate("/usr/bin/rm -rf /");
771+
assert!(!result.valid, "Full path should not bypass blocked command");
772+
773+
let result = validator.validate("./rm -rf /");
774+
assert!(!result.valid, "Relative path should not bypass blocked command");
775+
}
776+
777+
#[test]
778+
fn test_command_validation_combined_bypass() {
779+
let validator = CommandValidator::new();
780+
781+
// Combined bypass attempts
782+
let result = validator.validate("'/bin/rm' -rf /");
783+
assert!(!result.valid, "Combined path and whitespace should not bypass");
784+
785+
let result = validator.validate("\"/usr/bin/rm\" '-rf' '/'");
786+
assert!(!result.valid, "Combined quotes, path, and whitespace should not bypass");
787+
}
788+
789+
#[test]
790+
fn test_normalize_command() {
791+
// Test the normalize function directly
792+
assert_eq!(normalize_command("rm -rf /"), "rm -rf /");
793+
assert_eq!(normalize_command("rm -rf /"), "rm -rf /");
794+
assert_eq!(normalize_command("'rm' -rf /"), "rm -rf /");
795+
assert_eq!(normalize_command("/bin/rm -rf /"), "rm -rf /");
796+
assert_eq!(normalize_command("'/usr/bin/rm' '-rf' '/'"), "rm -rf /");
797+
}
798+
703799
#[test]
704800
fn test_url_validation() {
705801
let validator = UrlValidator::new();

0 commit comments

Comments
 (0)