# Vulnerability Verification Report

## 1. Summary
- **Vulnerability Type**: rce (CWE-78 / CWE-77 — OS Command Injection / Argument Injection)
- **Flagged Location**: `src/security/shell.rs:171-245` (`ShellSecurityConfig::validate_command`), with bypass-enabling definitions at `src/security/shell.rs:18-67` and `src/security/shell.rs:216-242`
- **Vulnerability Description**: The shell allowlist/blocklist in `ShellSecurityConfig::validate_command` can be fully bypassed through (a) command injection metacharacters (`;`, `&&`, `|`, `$()`) that defeat the first-token-only allowlist check, (b) argument injection (e.g. `python -P -c …`) that defeats the `python[23]?\s+-c\s+` regex, and (c) filename glob/wildcard expansion (e.g. `/etc/pass[w]d`) that defeats the literal-substring blocked-path list while the shell still resolves the path.

## 2. Analysis Logic

### Step 1: Read the flagged validator
`src/security/shell.rs:171-245` shows `validate_command` performs three sequential checks:

```rust
// Check regex patterns   (lines 179-196)
for pattern in &self.compiled_patterns {
    if pattern.is_match(command) { return Err(...); }
}
// Check literal patterns  (lines 199-213)
for literal in &self.literal_patterns {
    if command_lower.contains(literal) { return Err(...); }
}
// Allowlist check         (lines 216-242)
if self.allowlist_mode != ShellAllowlistMode::Off && !self.allowlist.is_empty() {
    let first_token = command.split_whitespace().next().unwrap_or("").to_lowercase();
    let executable = first_token.rsplit('/').next().unwrap_or(&first_token);
    if !self.allowlist.iter().any(|a| a == executable) {
        match self.allowlist_mode { Strict => return Err(...), … }
    }
}
```

All three checks operate on the raw command string; nothing tokenizes shell metacharacters. Next I traced whether the validator is the actual security boundary for runtime execution.

### Step 2: Trace the validator's callers to the execution sink
`ShellTool::execute` at `src/tools/shell.rs:144-174` is the primary caller:

```rust
let command = args.get("command").and_then(|v| v.as_str())
    .ok_or_else(|| ZeptoError::Tool("Missing 'command' argument".into()))?;
// Security check
self.security_config.validate_command(command)?;          // shell.rs:151
…
let output = self.runtime.execute(command, &container_config).await   // shell.rs:167-169
```

`args` is the LLM tool-call JSON; `command` is taken verbatim. The runtime sink is `NativeRuntime::execute` at `src/runtime/native.rs:35-41`:

```rust
let mut cmd = Command::new("sh");
cmd.arg("-c").arg(command);
```

So the command string is interpreted by `sh -c` — every metacharacter (`;`, `|`, `&&`, `$()`, `[]`, backticks) is live. `ShellTool::new()` (`src/tools/shell.rs:52-57`) defaults to `NativeRuntime` with no sandbox. The same validator is also used by `CustomTool::execute` (`src/tools/custom.rs:117`) before another `sh -c` invocation built at `src/tools/custom.rs:131-132` (`Command::new("sh").arg("-c").arg(&command)`).

A separate `validate_command_safety` exists at `src/plugins/loader.rs:365-392` and does reject `;`, `&&`, `||`, `|`, backticks — but it is applied **only to plugin command templates at load time** (`src/plugins/loader.rs:245`), not to the runtime `command` string passed to `ShellTool`/`CustomTool`. It therefore does not close the hole for the agent-driven shell tool. Next I verified each of the three claimed bypasses against the actual patterns.

### Step 3: Verify the allowlist bypass (first-token-only)
In `ShellAllowlistMode::Strict` with `allowlist = ["git"]`, the PoC's first token is `/usr/bin/git`. At `src/security/shell.rs:217-224` the code computes:

```rust
let first_token = command.split_whitespace().next().unwrap_or("").to_lowercase(); // "/usr/bin/git"
let executable = first_token.rsplit('/').next().unwrap_or(&first_token);          // "git"
if !self.allowlist.iter().any(|a| a == executable) { … }                          // git is in allowlist → pass
```

Nothing after the first whitespace token is checked against the allowlist, so `; python -P -c '…'; cat /etc/pass[w]d` is never re-validated. The first-token check therefore degrades to a cosmetic filter in the presence of `;`, `&&`, `||`, `|`, or `$()`. Confirmed bypass.

### Step 4: Verify the regex-blocklist bypass via argument injection
The relevant patterns at `src/security/shell.rs:43-46`:

```rust
r"python[23]?\s+-c\s+",
r"perl\s+-e\s+",
r"ruby\s+-e\s+",
r"node\s+-e\s+",
```

These require the interpreter token to be **immediately** followed by `-c`/`-e` (only `\s+` between). The PoC uses `python -P -c '…'`. After `python` + whitespace, the next token is `-P`, not `-c`, so `python[23]?\s+-c\s+` does not match. The other regexes (`eval\s+`, `base64\s+(-d|--decode)`, `xargs\s+.*sh\b`, etc.) are similarly positional and trivially evaded by inserting an inert flag (`-P`, `-X`, `--help`, etc.) between the binary and the dangerous flag. Confirmed bypass.

### Step 5: Verify the literal-blocklist bypass via wildcards
`src/security/shell.rs:56-67` defines `LITERAL_BLOCKED_PATTERNS` including `"/etc/passwd"`. The match is a plain substring test at `src/security/shell.rs:199-200`:

```rust
for literal in &self.literal_patterns {
    if command_lower.contains(literal) { … }
}
```

The PoC uses `cat /etc/pass[w]d`. The lowercased command contains `/etc/pass[w]d` but **not** the substring `/etc/passwd`, so the literal check passes. When `sh -c` later runs the command, the `[w]` glob expands to `w`, so `cat` reads `/etc/passwd` exactly. Any bracket-glob (`[r][o][o][t]`), `?`, or unquoted variable (`/etc/pass$'w'd`) achieves the same effect for every literal in the list (`~/.ssh/`, `.aws/credentials`, `.kube/config`, etc.). Confirmed bypass.

### Step 6: Confirm the PoC command passes the validator end-to-end
Walking the PoC `/usr/bin/git status; python -P -c 'import os; os.system("rm -rf /")'; cat /etc/pass[w]d` through every check in `validate_command`:

- Regex patterns (`src/security/shell.rs:18-53`):
  - `python[23]?\s+-c\s+` — `python -P -c` has `-P` between `python` and `-c` → no match.
  - `rm\s+(-[rf]{1,2}\s+)*(-[rf]{1,2}\s+)*/\s*($|;|\||&)` — `rm -rf /")` has `"` after `/`, not `($|;|\||&)` → no match.
  - `curl/wget | sh`, `/dev/tcp/`, `mkfs`, `dd …`, `chmod 777`, fork-bomb, `base64 -d`, `perl -e`, `ruby -e`, `node -e`, `eval\s+`, `xargs … sh`, `env >` — none present.
- Literal patterns (`src/security/shell.rs:56-67`): `/etc/pass[w]d` does not contain `/etc/passwd` (or any other literal) as a substring → no match.
- Allowlist (`src/security/shell.rs:216-242`): first token `/usr/bin/git` → executable `git` → in allowlist → pass.

`validate_command` returns `Ok(())`. `ShellTool::execute` then hands the string to `NativeRuntime::execute`, which runs `sh -c "/usr/bin/git status; python -P -c 'import os; os.system(\"rm -rf /\")'; cat /etc/pass[w]d"`, executing `git status`, then `python -c` that runs `rm -rf /`, then `cat /etc/passwd`. All three guards defeated by a single command string. The bypass is real and reachable from the agent tool-call surface.

### Step 7: Secondary issue — empty allowlist in Strict mode silently disables the check
At `src/security/shell.rs:216` the guard is `if self.allowlist_mode != ShellAllowlistMode::Off && !self.allowlist.is_empty()`. In `ShellAllowlistMode::Strict` with an empty allowlist, the entire allowlist block is skipped, so every command that survives the blocklist is allowed. The blocklist is itself bypassable per Steps 4–5, so this collapses the security posture to "blocklist only" even when the operator configured `Strict`. The repo's own test `test_allowlist_strict_empty_blocks_nothing` (`src/security/shell.rs:516-525`) documents and accepts this behavior.

### Analysis Process
- Q1 (sink): Is user/agent-controlled input executed by a shell? → **YES**. `ShellTool::execute` takes `args["command"]` verbatim (`src/tools/shell.rs:145-151`) and passes it to `NativeRuntime::execute` which runs `sh -c <command>` (`src/runtime/native.rs:40-41`). Same pattern in `CustomTool::execute` (`src/tools/custom.rs:114, 131-132`).
- Q2 (sanitizer on path): Is there a tokenizer, shell-escaper, or metacharacter rejector between source and sink? → **NO**. The only check is `ShellSecurityConfig::validate_command`, which is a regex/literal/first-token matcher on the raw string (`src/security/shell.rs:171-245`). It does not parse shell grammar. The metacharacter-rejecting `validate_command_safety` (`src/plugins/loader.rs:365`) is applied only to plugin templates at load time (`src/plugins/loader.rs:245`), not to runtime commands.
- Q3 (allowlist effective): Does the allowlist confine the executable across the whole command? → **NO**. It inspects only `split_whitespace().next()` (`src/security/shell.rs:217-224`), so `;`, `&&`, `|`, `$()` chain additional executables unobserved.
- Q4 (blocklist robust to argument injection): Do the regex anchors prevent flag-position evasion? → **NO**. Patterns like `python[23]?\s+-c\s+` (`src/security/shell.rs:43`) require `-c` immediately after the interpreter; inserting any inert flag (e.g. `-P`) defeats them.
- Q5 (literal list robust to globbing): Does the literal check resist shell wildcard expansion? → **NO**. It uses `String::contains` on the raw command (`src/security/shell.rs:200`), so `/etc/pass[w]d` evades the `/etc/passwd` literal while `sh` still expands `[w]` to `w`.
- Q6 (framework/runtime protection by default): Is a sandboxed runtime used by default? → **NO**. `ShellTool::new()` constructs `NativeRuntime` (`src/tools/shell.rs:52-57`), which runs `sh -c` directly on the host. The `validate_command` blocklist/allowlist is therefore the primary, and bypassable, boundary for the default configuration.
- → Reached leaf: **TRUE_POSITIVE**

## 3. Verdict
**TRUE_POSITIVE**

**Key Evidence:**
- `src/security/shell.rs:217-224` — allowlist checks only `split_whitespace().next()` and strips path prefix, so `;`/`&&`/`$()` chain arbitrary additional commands past the allowlist in `Strict` mode.
- `src/security/shell.rs:43-46` — regex blocklist requires `-c`/`-e` immediately after the interpreter token (`python[23]?\s+-c\s+`), so argument injection like `python -P -c '…'` evades it.
- `src/security/shell.rs:56-67, 199-200` — literal blocklist uses `command_lower.contains(literal)`; glob/wildcard spellings such as `/etc/pass[w]d` are not substring-matched, while `sh -c` still expands them to the sensitive path.
- `src/tools/shell.rs:145-151` and `src/runtime/native.rs:40-41` — the bypassed command flows verbatim from the agent tool-call args to `sh -c <command>` on the host with no shell-escaping or metacharacter rejection in between.
- `src/security/shell.rs:216` — `&& !self.allowlist.is_empty()` silently disables the allowlist in `Strict` mode when the allowlist is empty, compounding the bypass.

## 4. Remediation Suggestions
- Treat `validate_command` as defense-in-depth only (as the file's own doc-comment at `src/security/shell.rs:11-17` already implies) and make a real isolation boundary the default for `ShellTool`: prefer `DockerRuntime`/`AppleContainerRuntime`/`LandlockRuntime` over `NativeRuntime`, or require an explicit opt-in to run on the host.
- Replace the regex/literal/first-token matcher with a real shell-grammar-aware policy. Concretely, reject (or properly quote) shell metacharacters — `;`, `&&`, `||`, `|`, `$()`, backticks, `>`, `<`, `&`, `*`, `?`, `[`, `]`, `{`, `}`, `~` — before execution, mirroring the existing `validate_command_safety` at `src/plugins/loader.rs:365-392` but applying it at runtime to `ShellTool`/`CustomTool` commands, not just plugin templates.
- For the allowlist: parse the command into argv (e.g. via `shell-words`), then enforce that **every** invoked executable (across all `;`/`|`/`&&` segments) is in the allowlist — or, better, bypass the shell entirely by executing `Command::new(executable).args(argv)` instead of `sh -c <string>` so no metacharacter interpretation happens.
- For the regex blocklist: anchor on token boundaries, not whitespace adjacency. Match the dangerous flag anywhere in the argv list (e.g. `\bpython[23]?\b(?!.*\s-c\b)` style or, preferably, argv-level inspection after parsing).
- For the literal blocklist: canonicalize/glob-expand the argument list before matching, or block any argument containing glob metacharacters (`[`, `]`, `?`, `*`) when it targets a sensitive path. At minimum also block bracket-glob spellings of every literal (e.g. `/etc/pass\[w\]d` and `/etc/pass?wd`).
- Fix the empty-allowlist edge case at `src/security/shell.rs:216`: in `ShellAllowlistMode::Strict`, an empty allowlist should reject all commands, not skip the check. Change the guard to `if self.allowlist_mode != ShellAllowlistMode::Off` and handle the empty-list case explicitly inside.
- Add regression tests for the PoC (`/usr/bin/git status; python -P -c '…'; cat /etc/pass[w]d`) and the three individual bypass classes (command-injection chaining, argument injection, wildcard literal evasion) so future pattern tweaks do not silently reintroduce them.
