# Vulnerability Verification Report

## 1. Summary
- **Vulnerability Type**: rce (CWE-78 / CWE-77 — argument-permutation blocklist bypass)
- **Flagged Location**: `src/tools/android/actions.rs:413-424`
- **Vulnerability Description**: The Android `device_shell` blocklist matches dangerous commands as literal substring patterns (e.g. `rm -rf`, `rm -r`, `dd if=`) after whitespace normalization. Because argument order is not normalized, an attacker can permute flags (e.g. `rm -fr`, `rm --recursive --force`, `dd of=... if=...`) to bypass the blocklist and execute destructive commands on the connected Android device.

## 2. Analysis Logic

### Step 1: Examining the flagged sink at `src/tools/android/actions.rs:408-446`
Read the `device_shell` function. The blocklist is applied as substring `contains` checks after `split_whitespace().join(" ")` normalization and `to_lowercase()`:

```rust
pub async fn device_shell(adb: &AdbExecutor, cmd: &str) -> Result<String> {
    let normalized: String = cmd.split_whitespace().collect::<Vec<_>>().join(" ");
    let lower = normalized.to_lowercase();

    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(...);
        }
    }
    ...
    adb.shell(cmd).await
}
```

The normalization collapses whitespace and lowercases, but does NOT canonicalize argument order or expand short-flag combinations. Any multi-token pattern (`rm -rf`, `rm -r`, `dd if=`) is matchable only in that exact token sequence.

### Step 2: Tracing `device_shell` callers
`Grep` for `device_shell` across the project found one production caller at `src/tools/android/mod.rs:260`:

```rust
"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
}
```

The `command` string is taken directly from the tool arguments JSON (the `command` property) with no extra normalization, validation, or allowlisting before being passed to `device_shell`. The blocklist inside `device_shell` is the only security boundary.

### Step 3: Tracing the tool entry point at `src/tools/android/mod.rs`
The `AndroidTool::execute` dispatch matches `"shell"` action and forwards the raw `command` arg. The `AndroidTool` is gated behind the `android` Cargo feature (`Cargo.toml:202`: `android = ["quick-xml"]`); when enabled, the tool is registered for the agent and is callable by the LLM. The agent's tool input is treated as trusted-enough to be executed on a real connected device via `adb shell`, so the blocklist is the security gate that prevents destructive device-side operations.

### Step 4: Demonstrating the bypass concretely
Walking through the matching logic for permuted arguments:

- Input `rm -fr /sdcard`:
  - `normalized = "rm -fr /sdcard"`, `lower = "rm -fr /sdcard"`.
  - `"rm -rf"` substring? No — char sequence is `r m <space> - f r`, the `f` precedes the `r`.
  - `"rm -r"` substring? No — after `rm ` comes `-fr`; the character after `-` is `f`, not `r`.
  - No other patterns match. → Blocklist bypassed; `adb shell rm -fr /sdcard` is executed.

- Input `rm --recursive --force /sdcard`: no blocked substring matches. → Bypassed.
- Input `dd of=/dev/block/mmcblk0 if=/dev/zero`: `"dd if="` requires `dd if=` contiguously; here `dd` is followed by `of=`. → Bypassed.

Note: the variant `rm -r -f` IS still caught because `"rm -r"` is a substring of `"rm -r -f"`. The advisory's exact example is correct that other permutations (notably `rm -fr`) bypass.

### Step 5: Checking for additional sanitizers
Searched the function and its callers — there is no argument parser, no flag-canonicalization step, no allowlist of permitted commands, and no further validation between `device_shell` and `adb.shell(cmd)`. The metacharacter filter (`;`, `|`, `` ` ``, `$`, `&`, `\n` at `actions.rs:435`) is orthogonal — it blocks chaining but does not catch flag permutations of a single command. There is no other defense in depth.

### Step 6: Checking tests for intended coverage
`src/tools/android/actions.rs:745-752` (`test_device_shell_blocks_normalized_whitespace`) only verifies that extra whitespace between `rm` and `-rf` is caught (`rm   -rf /sdcard` → `"rm -rf"` after normalization). It does not test permuted flag order. No test asserts that `rm -fr` or `rm --recursive` are blocked. This confirms the bypass is an unhandled case, not an intentional design decision.

### Analysis Process
- Q1: Is the dangerous sink reachable from a user/agent-controllable input? → YES. `device_shell` is invoked from `AndroidTool::execute` action `"shell"` with the `command` parameter taken verbatim from tool args (`src/tools/android/mod.rs:255-260`).
- Q2: Does the blocklist perform effective canonicalization of arguments? → NO. It only normalizes whitespace and case (`src/tools/android/actions.rs:410-411`); token order is preserved.
- Q3: Can the blocklist be bypassed by argument permutation? → YES. `rm -fr /sdcard`, `rm --recursive --force`, and `dd of=... if=...` all evade the substring patterns (`rm -rf`, `rm -r`, `dd if=`) as shown in Step 4.
- Q4: Is there any other sanitizer/validator on the path? → NO. The metacharacter filter only blocks `;|`$&\n` and does not affect flag ordering. No allowlist or argument parser exists (`src/tools/android/actions.rs:434-443`).
- Q5: Is this dead/test/example code? → NO. `device_shell` is the production handler for the `shell` action on the registered `AndroidTool`, reachable whenever the `android` feature is enabled.
- → Reached leaf: TRUE_POSITIVE

## 3. Verdict
**TRUE_POSITIVE**

**Key Evidence:**
- `src/tools/android/actions.rs:413-432` — blocklist matches multi-token patterns (`rm -rf`, `rm -r`, `dd if=`) as literal substrings; whitespace/case are normalized but token order is not.
- `src/tools/android/mod.rs:255-260` — `device_shell` is the production handler for the agent-callable `shell` action, receiving the raw `command` argument with no additional sanitization.
- `src/tools/android/actions.rs:745-752` — the only whitespace-normalization test covers `rm   -rf`, not permuted orders like `rm -fr`, confirming the bypass is unhandled.

## 4. Remediation Suggestions
Replace substring matching with token-aware, order-insensitive blocking:

1. **Tokenize and canonicalize**: split the command into `program + args`, then for each blocked program check for the *presence* of dangerous flags regardless of order, e.g. reject `rm` when any of `-r`, `-R`, `-f`, `--recursive`, `--force`, `-rf`, `-fr`, `-Rf`, `-fR` appear (including combined short-flag clusters). Parse combined short flags so `-fr` is treated as `-f -r`.
2. **Allowlist instead of blocklist**: prefer an explicit allowlist of permitted `device_shell` commands (e.g. `ls`, `cat`, `pwd`, `getprop`) and reject everything else. Blocklists for shell commands are inherently bypass-prone.
3. **Cover the `dd` case**: for `dd`, block the program entirely (or block any `of=/dev/block*` / `of=/dev/sd*` / `of=/dev/mmc*` references), not just the `dd if=` substring.
4. **Add regression tests** for permuted orders: `rm -fr`, `rm --recursive --force`, `rm -r -f` (this one already passes), `dd of=... if=...`, and combined short flags like `rm -rf` vs `rm -fr`.
5. **Re-check after fix**: ensure the agent's tool-result sanitization (`src/utils/sanitize.rs`) and the metacharacter filter remain in place as defense-in-depth; the blocklist should not be the only boundary for destructive device operations.
