Skip to content

Commit 762e207

Browse files
πŸ”’ Fix command injection vulnerability in command_exists
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent 598b5a2 commit 762e207

7 files changed

Lines changed: 137 additions & 1 deletion

File tree

β€Ž.jules/sentinel.mdβ€Ž

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,23 @@
22
**Vulnerability:** Path traversal existed in both the Python (`src/session_store.py`) and Rust (`rust/crates/runtime/src/session_control.rs`) implementations because unsanitized `session_id` strings were used directly in file paths.
33
**Learning:** Both reference implementations lacked central validation logic for system identifiers derived from external/user input.
44
**Prevention:** Always validate and restrict identifier parameters (like session IDs) by checking for explicit disallow-lists (like path separators `/`, `\`, and directory traversal markers `.`, `..`) before using them in file operations.
5+
6+
## 2025-02-24 - [Command Injection via String Formatting in Shell Invocations]
7+
8+
**Vulnerability:**
9+
Using `format!` or string concatenation to build commands passed to `sh -c` directly injects user-controlled input into the shell environment without proper sanitization, enabling command injection (e.g., passing `nonexistent; echo injected`).
10+
11+
**Learning:**
12+
Even if commands seem innocuous or are just fetching basic system properties, formatting strings directly into `-c` shell flags compromises security in Rust when input comes from users or dynamic sources.
13+
14+
**Prevention:**
15+
Always use positional shell arguments to safely handle user input. Instead of `sh -c format!("cmd {}", user_input)`, write `sh -c 'cmd "$1"' -- user_input`.
16+
For example, in Rust's `Command` builder:
17+
```rust
18+
std::process::Command::new("sh")
19+
.arg("-c")
20+
.arg("command -v \"$1\" >/dev/null 2>&1")
21+
.arg("--")
22+
.arg(command)
23+
```
24+
This guarantees the shell interprets the user input purely as a string argument and not as executable commands.

β€Žappend_test.pyβ€Ž

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import re
2+
3+
with open("rust/crates/tools/src/lib.rs", "r") as f:
4+
content = f.read()
5+
6+
test_content = """ #[test]
7+
fn test_command_exists_injection_prevention() {
8+
// Try to inject a command
9+
let injected_command = "nonexistent_command; echo 'injected'";
10+
11+
// This should return false and NOT execute the injected command
12+
assert!(!command_exists(injected_command));
13+
}
14+
}"""
15+
16+
content = re.sub(r'}\s*$', test_content + '\n', content)
17+
18+
with open("rust/crates/tools/src/lib.rs", "w") as f:
19+
f.write(content)
20+
print("Appended successfully")

β€Žfix_command_exists.patchβ€Ž

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--- rust/crates/tools/src/lib.rs
2+
+++ rust/crates/tools/src/lib.rs
3+
@@ -5150,8 +5150,9 @@
4+
5+
fn command_exists(command: &str) -> bool {
6+
std::process::Command::new("sh")
7+
.arg("-c")
8+
- .arg(format!("command -v {command} >/dev/null 2>&1"))
9+
+ .arg("command -v \"$1\" >/dev/null 2>&1")
10+
+ .arg("--")
11+
+ .arg(command)
12+
.status()
13+
.map(|status| status.success())
14+
.unwrap_or(false)

β€Žfix_command_exists.pyβ€Ž

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import re
2+
3+
with open("rust/crates/tools/src/lib.rs", "r") as f:
4+
content = f.read()
5+
6+
old_code = """fn command_exists(command: &str) -> bool {
7+
std::process::Command::new("sh")
8+
.arg("-c")
9+
.arg("command -v "$1" >/dev/null 2>&1")
10+
.arg("--")
11+
.arg(command)
12+
.status()
13+
.map(|status| status.success())
14+
.unwrap_or(false)
15+
}"""
16+
17+
new_code = """fn command_exists(command: &str) -> bool {
18+
std::process::Command::new("sh")
19+
.arg("-c")
20+
.arg("command -v \\\"$1\\\" >/dev/null 2>&1")
21+
.arg("--")
22+
.arg(command)
23+
.status()
24+
.map(|status| status.success())
25+
.unwrap_or(false)
26+
}"""
27+
28+
if old_code in content:
29+
content = content.replace(old_code, new_code)
30+
with open("rust/crates/tools/src/lib.rs", "w") as f:
31+
f.write(content)
32+
print("Replaced successfully")
33+
else:
34+
print("Could not find the target codeblock.")

β€Žfix_test_import.pyβ€Ž

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import re
2+
3+
with open("rust/crates/tools/src/lib.rs", "r") as f:
4+
content = f.read()
5+
6+
content = content.replace("assert!(!command_exists(injected_command));", "assert!(!crate::command_exists(injected_command));")
7+
8+
with open("rust/crates/tools/src/lib.rs", "w") as f:
9+
f.write(content)
10+
print("Fixed successfully")

β€Žrevert_tests.pyβ€Ž

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import re
2+
3+
with open("rust/crates/tools/src/lib.rs", "r") as f:
4+
content = f.read()
5+
6+
bad_content = """
7+
#[cfg(test)]
8+
mod tests {
9+
use super::*;
10+
11+
#[test]
12+
fn test_command_exists_injection_prevention() {
13+
// Try to inject a command
14+
let injected_command = "nonexistent_command; echo 'injected'";
15+
16+
// This should return false and NOT execute the injected command
17+
assert!(!command_exists(injected_command));
18+
}
19+
}
20+
"""
21+
22+
if bad_content in content:
23+
content = content.replace(bad_content, "")
24+
with open("rust/crates/tools/src/lib.rs", "w") as f:
25+
f.write(content)
26+
print("Reverted successfully")
27+
else:
28+
print("Could not find bad content")

β€Žrust/crates/tools/src/lib.rsβ€Ž

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5151,7 +5151,9 @@ fn detect_powershell_shell() -> std::io::Result<&'static str> {
51515151
fn command_exists(command: &str) -> bool {
51525152
std::process::Command::new("sh")
51535153
.arg("-c")
5154-
.arg(format!("command -v {command} >/dev/null 2>&1"))
5154+
.arg("command -v \"$1\" >/dev/null 2>&1")
5155+
.arg("--")
5156+
.arg(command)
51555157
.status()
51565158
.map(|status| status.success())
51575159
.unwrap_or(false)
@@ -8583,4 +8585,12 @@ printf 'pwsh:%s' "$1"
85838585
.into_bytes()
85848586
}
85858587
}
8588+
#[test]
8589+
fn test_command_exists_injection_prevention() {
8590+
// Try to inject a command
8591+
let injected_command = "nonexistent_command; echo 'injected'";
8592+
8593+
// This should return false and NOT execute the injected command
8594+
assert!(!crate::command_exists(injected_command));
8595+
}
85868596
}

0 commit comments

Comments
Β (0)