# Vulnerability Verification Report

## 1. Summary
- **Vulnerability Type**: Remote Code Execution — Shell command injection via blocklist bypass (CWE-184: Incomplete List of Disallowed Inputs)
- **Flagged Location**: `src/tools/android/actions.rs:408-446`
- **Vulnerability Description**: The `device_shell()` function implements a substring-based blocklist to prevent dangerous shell commands on Android devices, but the blocklist can be bypassed by permuting argument order (e.g., `rm -f -r` or `rm -fr` instead of `rm -rf`).

## 2. Analysis Logic

### Step 1: Examining the flagged sink — `device_shell()` in `src/tools/android/actions.rs:408-446`

Read the full function. The blocklist at lines 413-424 uses simple substring matching:

```rust
// src/tools/android/actions.rs:413-424
let blocked = [
    "rm -rf",
    "rm -r",
    "reboot",
    "factory_reset",
    "wipe",
    "format",
    "dd if=",
    "mkfs",
    "flash",
    "fastboot",
];
for pattern in &blocked {
    if lower.contains(pattern) {
        return Err(ZeptoError::Tool(format!(
            "Blocked dangerous command containing '{}'",
            pattern
        )));
    }
}
```

It also blocks shell metacharacters at lines 435-443:

```rust
// src/tools/android/actions.rs:435-443
let dangerous_chars = [';', '|', '`', '$', '&', '\n'];
for ch in &dangerous_chars {
    if cmd.contains(*ch) {
        return Err(ZeptoError::Tool(format!(
            "Blocked shell metacharacter '{}' in command. Use specific actions instead of raw shell.",
            ch
        )));
    }
}
```

After both checks pass, the command is executed directly on the Android device at line 445:

```rust
// src/tools/android/actions.rs:445
adb.shell(cmd).await
```

**Analysis**: The blocklist is substring-based. Checking `lower.contains("rm -rf")` and `lower.contains("rm -r")` does NOT catch `rm -f -r` or `rm -fr`, because those strings do not contain `"rm -rf"` or `"rm -r"` as contiguous substrings. The metacharacter check blocks command chaining but does not prevent argument permutation.

### Step 2: Tracing the command flow — how input reaches `device_shell()`

The `AndroidTool` struct implements the `Tool` trait and is registered with the agent:

- `src/tools/android/mod.rs:361-371` — `execute()` method receives JSON `args` from the LLM agent:

```rust
// src/tools/android/mod.rs:361-371
async fn execute(&self, args: Value, _ctx: &ToolContext) -> Result<ToolOutput> {
    let action = args
        .get("action")
        .and_then(|v| v.as_str())
        .ok_or_else(|| ZeptoError::Tool("Missing 'action' parameter".into()))?;

    debug!(action = action, "Android tool executing");
    self.dispatch_action(action, &args)
        .await
        .map(ToolOutput::llm_only)
}
```

- `src/tools/android/mod.rs:255-261` — The `"shell"` action case extracts the `command` parameter and passes it to `device_shell()`:

```rust
// src/tools/android/mod.rs:255-261
"shell" => {
    let cmd = args
        .get("command")
        .and_then(|v| v.as_str())
        .ok_or_else(|| ZeptoError::Tool("Missing 'command' parameter".into()))?;
    actions::device_shell(&self.adb, cmd).await
}
```

- `src/tools/android/adb.rs:77-79` — `device_shell` calls `adb.shell(cmd)`:

```rust
// src/tools/android/adb.rs:77-79
pub async fn shell(&self, cmd: &str) -> Result<String> {
    self.run(&["shell", cmd]).await
}
```

- `src/tools/android/adb.rs:50-74` — `run()` passes the command as an argument to the `adb` subprocess (i.e., `adb shell <user_command>`), meaning the Android device's shell interprets the full command string:

```rust
// src/tools/android/adb.rs:62-63
Command::new(&self.adb_path).args(&cmd_args).output(),
```

**Analysis**: The full data flow is: LLM agent → `execute(args)` → `dispatch_action("shell")` → `device_shell(cmd)` → `adb.shell(cmd)` → Android device shell execution. The `command` parameter is fully controlled by what the LLM agent decides to pass — which can be influenced by users through chat channels (Telegram, Slack, Discord, etc.) via prompt injection or direct instructions.

### Step 3: Verifying blocklist bypass with concrete examples

After whitespace normalization (line 410: `cmd.split_whitespace().collect::<Vec<_>>().join(" ")`) and lowercasing (line 411):

| Command | Normalized | `"rm -rf"` contained? | `"rm -r"` contained? | Blocked? |
|---|---|---|---|---|
| `rm -rf /path` | `rm -rf /path` | YES | YES | YES |
| `rm -r /path` | `rm -r /path` | NO | YES | YES |
| `rm -r -f /path` | `rm -r -f /path` | NO | YES | YES |
| **`rm -f -r /path`** | `rm -f -r /path` | **NO** | **NO** | **NO** |
| **`rm -fr /path`** | `rm -fr /path` | **NO** | **NO** | **NO** |

For `rm -f -r /path`: after normalization it becomes the string `r`, `m`, ` `, `-`, `f`, ` `, `-`, `r`, ` `, `/`, `p`, `a`, `t`, `h`. The substring `"rm -r"` (`r`, `m`, ` `, `-`, `r`) does not appear because at position 4 the character is `f` not `r`. Similarly, `"rm -rf"` does not appear because there is no contiguous `r` and `f` after the space-plus-dash.

Both bypassing commands contain no blocked metacharacters (`;`, `|`, `` ` ``, `$`, `&`, `\n`), so they pass the metacharacter check at line 435-443 as well.

### Step 4: Checking for other security layers — the regex-based `ShellSecurityConfig`

The project already has a robust `ShellSecurityConfig` in `src/security/shell.rs:28-29` that uses regex patterns to handle flag variations:

```rust
// src/security/shell.rs:28-29
r"rm\s+(-[rf]{1,2}\s+)*(-[rf]{1,2}\s+)*/\s*($|;|\||&)",
```

This regex correctly blocks all flag permutations of `rm` targeting root: `rm -rf /`, `rm -fr /`, `rm -r -f /`, `rm -f -r /`, etc. The accompanying tests at `src/security/shell.rs:224-231` confirm this:

```rust
// src/security/shell.rs:224-231
#[test]
fn test_rm_rf_flag_variations() {
    let config = ShellSecurityConfig::new();
    // Different flag orderings
    assert!(config.validate_command("rm -r -f /").is_err());
    assert!(config.validate_command("rm -f -r /").is_err());
    assert!(config.validate_command("rm --recursive --force /").is_ok());
}
```

**However**, `device_shell()` in `src/tools/android/actions.rs` does NOT import or use `ShellSecurityConfig`. The general `ShellTool` in `src/tools/shell.rs:14` imports and uses it at line 151:

```rust
// src/tools/shell.rs:14
use crate::security::ShellSecurityConfig;
// ...
// src/tools/shell.rs:151
self.security_config.validate_command(command)?;
```

But the Android tool has its own independent, weaker blocklist implementation. There is no reference to `ShellSecurityConfig` anywhere in `src/tools/android/`. The Android `device_shell` function does not benefit from the regex-based protections that the general shell tool enjoys.

### Step 5: Checking the approval gate coverage

The approval system at `src/tools/approval.rs:321-326` defines default dangerous tools:

```rust
// src/tools/approval.rs:321-326
pub fn default_dangerous_tools() -> Vec<String> {
    vec![
        "shell".to_string(),
        "write_file".to_string(),
        "edit_file".to_string(),
    ]
}
```

The `"android"` tool is NOT in the dangerous tools list. Even if a user enables the `RequireForDangerous` approval policy, the Android tool — including its `shell` action — would execute without requiring user approval.

### Step 6: Code context verification

- The Android tool module is gated behind `#[cfg(feature = "android")]` in `src/tools/mod.rs:57-58` and at the registration site in `src/cli/common.rs:885-891`. However, this is a legitimate feature flag for optional functionality, not a test/stub context. When the feature is enabled, the tool is fully operational.
- The code is NOT in a test directory, demo, example, or generated code.
- The tool is registered to the agent and available to LLM function calling.

### Analysis Process — RCE Decision Tree

- **Q1: Does user-controllable input reach a command execution sink?** → **YES**. The LLM agent provides the `command` parameter in the `android` tool's JSON args, which flows through `dispatch_action()` → `device_shell()` → `adb.shell(cmd)` → Android device shell execution (`adb shell <cmd>`). Users interacting through chat channels (Telegram, Slack, Discord) can induce the agent to issue arbitrary shell commands on Android devices. Evidence: `src/tools/android/mod.rs:255-261` and `src/tools/android/actions.rs:445`.

- **Q2: Is it code evaluation (eval/exec) with user data?** → **No**. It is process execution via the `adb` subprocess invoking the Android device's shell.

- **Q3: Is the command executed in array form without shell?** → **No**. The command is passed as a single string to `adb shell`, which invokes the Android device's shell to interpret the full command. Evidence: `src/tools/android/adb.rs:78` passes `cmd` as a single string to `self.run(&["shell", cmd])`.

- **Q4: Is the user input effectively sanitized?** → **No**. The Android tool's `device_shell()` uses a substring-based blocklist that can be bypassed via argument permutation (e.g., `rm -f -r`, `rm -fr`). The project has a robust regex-based `ShellSecurityConfig` that handles these cases correctly, but `device_shell` does not use it. Evidence: `src/tools/android/actions.rs:413-432` vs. `src/security/shell.rs:28-29`.

- **Q5: Is the code in test/demo/dead/generated context?** → **No**. It is production code registered as a tool to the agent. Evidence: `src/cli/common.rs:885-891`.

→ **Reached leaf: TRUE_POSITIVE**

## 3. Verdict
**TRUE_POSITIVE**

**Key Evidence:**
- The blocklist at `src/tools/android/actions.rs:413-424` uses substring matching that fails to catch `rm -f -r` or `rm -fr` argument permutations (confirmed by character-by-character substring analysis)
- The robust regex-based `ShellSecurityConfig` at `src/security/shell.rs:28-29` correctly handles all flag variations, but `device_shell()` does not import or use it (confirmed by full file reads of both `actions.rs` and `shell.rs`)
- The approval gate at `src/tools/approval.rs:321-326` lists `"shell"` but not `"android"` in its default dangerous tools, so even the `RequireForDangerous` policy would not prevent this
- The `adb.shell(cmd)` call at `src/tools/android/adb.rs:78` passes the user-controlled string to the Android device's shell, which interprets it with full shell semantics

## 4. Remediation Suggestions

**Recommended fix**: Replace the substring-based blocklist in `device_shell()` with the existing `ShellSecurityConfig::validate_command()` from `src/security/shell.rs`. This would unify the security logic and immediately close the argument-permutation bypasses.

Specifically, in `src/tools/android/actions.rs`, the `device_shell()` function should:
1. Import `crate::security::ShellSecurityConfig`
2. Call `ShellSecurityConfig::new().validate_command(cmd)` instead of (or in addition to) the current substring matching
3. Retain the metacharacter check (`;`, `|`, `` ` ``, `$`, `&`, `\n`) as it provides defense-in-depth against established shell injection vectors

Additionally:
- Add `"android"` to the default dangerous tools list at `src/tools/approval.rs:321-326` so the `RequireForDangerous` approval policy covers it
- Consider adding a test case for `rm -f -r` and `rm -fr` to ensure the fix prevents these specific bypasses
- Consider whether the `device_shell` action should be more restricted (e.g., only allow specific sub-commands rather than arbitrary shell input)
