fix(security): reject shell-unsafe chars in screenshot filename (#625)
Closes #601 The Linux screenshot path uses sh -c with single-quote interpolation. A filename containing quote characters could break quoting and inject shell tokens. Add a check that rejects filenames with any shell-breaking characters (quotes, backticks, dollar signs, semicolons, pipes, etc.) before passing to the shell command. Severity: High — command injection in tool execution path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
217a700bfa
commit
290d971d5e
1 changed files with 25 additions and 0 deletions
|
|
@ -68,6 +68,17 @@ impl ScreenshotTool {
|
||||||
|n| n.to_string_lossy().to_string(),
|
|n| n.to_string_lossy().to_string(),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Reject filenames with shell-breaking characters to prevent injection in sh -c
|
||||||
|
const SHELL_UNSAFE: &[char] =
|
||||||
|
&['\'', '"', '`', '$', '\\', ';', '|', '&', '\n', '\0', '(', ')'];
|
||||||
|
if safe_name.contains(SHELL_UNSAFE) {
|
||||||
|
return Ok(ToolResult {
|
||||||
|
success: false,
|
||||||
|
output: String::new(),
|
||||||
|
error: Some("Filename contains characters unsafe for shell execution".into()),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
let output_path = self.security.workspace_dir.join(&safe_name);
|
let output_path = self.security.workspace_dir.join(&safe_name);
|
||||||
let output_str = output_path.to_string_lossy().to_string();
|
let output_str = output_path.to_string_lossy().to_string();
|
||||||
|
|
||||||
|
|
@ -288,6 +299,20 @@ mod tests {
|
||||||
assert!(!args.is_empty());
|
assert!(!args.is_empty());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn screenshot_rejects_shell_injection_filename() {
|
||||||
|
let tool = ScreenshotTool::new(test_security());
|
||||||
|
let result = tool
|
||||||
|
.execute(json!({"filename": "test'injection.png"}))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert!(!result.success);
|
||||||
|
assert!(result
|
||||||
|
.error
|
||||||
|
.unwrap()
|
||||||
|
.contains("unsafe for shell execution"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn screenshot_command_contains_output_path() {
|
fn screenshot_command_contains_output_path() {
|
||||||
let cmd = ScreenshotTool::screenshot_command("/tmp/my_screenshot.png").unwrap();
|
let cmd = ScreenshotTool::screenshot_command("/tmp/my_screenshot.png").unwrap();
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue