# Vulnerability Verification Report

## 1. Summary
- **Vulnerability Type**: Remote command execution / Android device shell command policy bypass (CWE-78)
- **Flagged Location**: `src/tools/android/actions.rs:413`
- **Vulnerability Description**: The Android `shell` action tries to block dangerous device-shell commands with literal substring patterns, but equivalent `rm` option permutations such as `rm -fr` or `rm -f -r` do not match those literals and can still reach `adb shell`.

## 2. Analysis Logic

### Step 1: Examining the flagged Android device shell sink at `src/tools/android/actions.rs:408`
The flagged function receives a command string, normalizes whitespace only, checks literal blocked substrings, checks a small metacharacter set, and then executes the original command through ADB:

```rust
src/tools/android/actions.rs:407-445
/// Run an arbitrary shell command on the device.
pub async fn device_shell(adb: &AdbExecutor, cmd: &str) -> Result<String> {
    // Normalize whitespace for blocklist check
    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(ZeptoError::Tool(format!(
                "Blocked dangerous command containing '{}'",
                pattern
            )));
        }
    }

    // Block shell metacharacters that enable command chaining
    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
            )));
        }
    }

    adb.shell(cmd).await
}
```

The sink is `adb.shell(cmd).await` at `src/tools/android/actions.rs:445`. The command being checked is only whitespace-normalized for blocklist comparison at `src/tools/android/actions.rs:409-411`; the executed value remains the original `cmd` at `src/tools/android/actions.rs:445`.

### Step 2: Checking whether the blocklist catches `rm` argument permutations
The only `rm` patterns in the blocklist are `"rm -rf"` and `"rm -r"`:

```rust
src/tools/android/actions.rs:413-416
let blocked = [
    "rm -rf",
    "rm -r",
```

The bypass command `rm -fr /sdcard` normalizes to the same token order (`rm -fr /sdcard`) under the whitespace-only normalization at `src/tools/android/actions.rs:409-411`, but it contains neither `"rm -rf"` nor `"rm -r"` from `src/tools/android/actions.rs:413-416`. The alternative `rm -f -r /sdcard` likewise contains neither literal. These strings also avoid the only metacharacters checked at `src/tools/android/actions.rs:434-437`:

```rust
src/tools/android/actions.rs:434-437
// Block shell metacharacters that enable command chaining
let dangerous_chars = [';', '|', '`', '$', '&', '\n'];
for ch in &dangerous_chars {
    if cmd.contains(*ch) {
```

So the sanitizer is not an effective allowlist or command parser; it is a literal substring blocklist that misses semantically equivalent `rm` option forms.

### Step 3: Following the sink into ADB execution at `src/tools/android/adb.rs:75`
The Android shell helper passes the command as the second argument to `adb shell`:

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

The lower-level runner uses a fixed host executable (`adb`) with argument-array execution:

```rust
src/tools/android/adb.rs:59-62
let output = tokio::time::timeout(
    self.timeout,
    Command::new(&self.adb_path).args(&cmd_args).output(),
)
```

This avoids host shell metacharacter expansion, but it does not make the device command safe: the user-controlled string still becomes the command argument to `adb shell` at `src/tools/android/adb.rs:75-78`, which is the Android device shell execution sink.

### Step 4: Tracing the `command` argument from the Android tool API to `device_shell()`
The Android tool exposes a `shell` action, extracts `args["command"]` as a string, and passes it directly into `actions::device_shell()`:

```rust
src/tools/android/mod.rs:255-260
"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 JSON schema advertises this parameter as a free-form shell command:

```rust
src/tools/android/mod.rs:348-350
"command": {
    "type": "string",
    "description": "Shell command for device shell action"
},
```

The tool’s `execute()` method only extracts the requested action and dispatches with the original JSON args:

```rust
src/tools/android/mod.rs:361-370
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)
}
```

There is no allowlist of safe Android shell commands between the tool API and `device_shell()`.

### Step 5: Tracing reachability from user-controlled prompts to tool arguments
In CLI single-message mode, the user-supplied `-m/--message` value becomes an inbound message and is processed by the agent:

```rust
src/cli/agent.rs:96-99
if let Some(msg) = message {
    // Single message mode
    let inbound = InboundMessage::new("cli", "user", "cli", &msg);
    let streaming = stream || config.agents.defaults.streaming;
```

```rust
src/cli/agent.rs:127-128
match agent.process_message(&inbound).await {
    Ok(response) => {
```

Interactive CLI input is also converted directly into an inbound message:

```rust
src/cli/agent.rs:151-169
match stdin.lock().read_line(&mut input) {
    ...
    let input = input.trim();
    ...
    // Process message
    let inbound = InboundMessage::new("cli", "user", "cli", input);
```

The bus message type stores the text as `content`:

```rust
src/bus/message.rs:8-18
/// Represents an incoming message from a channel (e.g., Telegram, Discord, etc.)
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct InboundMessage {
    /// The channel this message came from (e.g., "telegram", "discord")
    pub channel: String,
    ...
    /// The text content of the message
    pub content: String,
```

The agent feeds `msg.content` into the LLM context and passes registered tool definitions to the provider:

```rust
src/agent/loop.rs:492-501
// Build messages with history
let messages = self
    .context_builder
    .build_messages(&session.messages, &msg.content);

// Get tool definitions (short-lived read lock)
let tool_definitions = {
    let tools = self.tools.read().await;
    tools.definitions_with_options(self.config.agents.defaults.compact_tools)
};
```

```rust
src/agent/loop.rs:549-552
// Call LLM -- provider lock is NOT held during this await
let mut response = provider
    .chat(messages, tool_definitions, model, options.clone())
    .await?;
```

When the LLM returns a tool call, the raw JSON arguments are parsed and passed into the registry for execution:

```rust
src/agent/loop.rs:634-655
let tool_futures: Vec<_> = response
    .tool_calls
    .iter()
    .map(|tool_call| {
        let tools = Arc::clone(&self.tools);
        let ctx = tool_ctx.clone();
        let name = tool_call.name.clone();
        let id = tool_call.id.clone();
        let raw_args = tool_call.arguments.clone();
        ...
        async move {
            let args: serde_json::Value = match serde_json::from_str(&raw_args) {
```

```rust
src/agent/loop.rs:726-729
let (result, success) = {
    let tools_guard = tools.read().await;
    match tools_guard.execute_with_context(&name, args, &ctx).await {
```

Therefore, an operator or allowed channel user can ask the agent to invoke `android` with `{"action":"shell","command":"rm -fr /sdcard"}`, and that command string follows the path: user prompt → LLM tool call arguments → `AndroidTool::execute()` → `dispatch_action("shell")` → `device_shell()` → `adb.shell()`.

### Step 6: Checking channel reachability and access controls
Telegram messages from allowed users are accepted as text and published to the same inbound bus:

```rust
src/channels/telegram.rs:385-397
// Check allowlist with deny_by_default support
let allowed = if allowlist.is_empty() {
    !deny_by_default
} else {
    allowlist.contains(&user_id)
};
if !allowed {
    info!(
        "Telegram: User {} not in allowlist, ignoring message",
        user_id
    );
    return Ok(());
}
```

```rust
src/channels/telegram.rs:399-401
// Only process text messages
if let Some(text) = msg.text() {
    let chat_id = msg.chat.id.0.to_string();
```

```rust
src/channels/telegram.rs:486-501
// Create and publish the inbound message
let mut inbound =
    InboundMessage::new("telegram", &user_id, &chat_id, text);
...
if let Err(e) = bus.publish_inbound(inbound).await {
```

This confirms the vulnerable tool is reachable from normal agent input paths, not only from tests. Telegram can restrict senders via allowlist, but the code allows messages when the allowlist is empty and `deny_by_default` is false (`src/channels/telegram.rs:385-388`), and allowed users still control the text that drives the tool call path.

### Step 7: Checking registration, feature gating, and mode/approval protections
The Android module and export are compiled only when the `android` Cargo feature is enabled:

```rust
src/tools/mod.rs:57-58
#[cfg(feature = "android")]
pub mod android;
```

```rust
src/tools/mod.rs:91-92
#[cfg(feature = "android")]
pub use android::AndroidTool;
```

The feature exists and is documented in the build configuration:

```toml
Cargo.toml:181-202
[features]
default = []
...
# Android device control tool via ADB (adds quick-xml for UI hierarchy parsing)
android = ["quick-xml"]
```

When the feature is enabled, the agent registers the Android tool unless the active tool profile excludes it:

```rust
src/cli/common.rs:884-890
// Register Android tool (feature-gated)
#[cfg(feature = "android")]
if tool_enabled("android") {
    agent
        .register_tool(Box::new(zeptoclaw::tools::android::AndroidTool::new()))
        .await;
    info!("Registered android tool");
}
```

The default tool profile is `None`, meaning all tools are eligible unless the user configures a profile:

```rust
src/config/types.rs:555-569
impl Default for AgentDefaults {
    fn default() -> Self {
        Self {
            ...
            compact_tools: false,
            tool_profile: None,
            timezone: default_timezone(),
        }
    }
}
```

The Android tool category is `Hardware`:

```rust
src/tools/android/mod.rs:292-295
fn category(&self) -> ToolCategory {
    ToolCategory::Hardware
}
```

Agent mode is not a default mitigation because the default mode is `Autonomous`, which allows all categories:

```rust
src/security/agent_mode.rs:16-30
/// Agent execution mode controlling tool permissions.
...
/// Determines which tool categories are allowed, require approval, or are
/// blocked during an agent session. The default mode is `Autonomous` for
/// backward compatibility.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)]
...
#[default]
Autonomous,
```

```rust
src/security/agent_mode.rs:89-93
pub fn check(&self, category: ToolCategory) -> CategoryPermission {
    match self.mode {
        AgentMode::Autonomous => CategoryPermission::Allowed,
```

The approval system is also disabled by default and defaults to always allow:

```rust
src/tools/approval.rs:94-100
/// # Defaults
///
/// - `enabled`: `false`
/// - `policy`: `AlwaysAllow`
/// - `require_for`: empty
/// - `dangerous_tools`: `["shell", "write_file", "edit_file"]`
```

```rust
src/tools/approval.rs:123-131
impl Default for ApprovalConfig {
    fn default() -> Self {
        Self {
            enabled: false,
            policy: ApprovalPolicyConfig::AlwaysAllow,
            require_for: Vec::new(),
            dangerous_tools: ApprovalGate::default_dangerous_tools(),
            auto_approve_timeout_secs: 0,
        }
    }
}
```

Additionally, the default dangerous tool list does not include the `android` tool:

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

So in the documented Android-enabled build, default mode and approval settings do not stop the Android `shell` action before the flawed blocklist.

### Analysis Process
- Q1: Does user-controllable input reach a command/code execution sink? → YES. CLI input becomes `InboundMessage::new(..., &msg)` at `src/cli/agent.rs:96-99`, `msg.content` is sent to the LLM with tool definitions at `src/agent/loop.rs:492-552`, returned tool arguments are executed at `src/agent/loop.rs:634-729`, and the Android `command` parameter reaches `adb.shell(cmd)` at `src/tools/android/mod.rs:255-260` and `src/tools/android/actions.rs:445`.
- Q2: Is it code evaluation (`eval`/`exec`) with user data? → NO. The sink is process/device-shell execution through `adb shell`, shown by `self.run(&["shell", cmd])` at `src/tools/android/adb.rs:75-78`.
- Q3: Is the command executed in array form without shell? → NO for the relevant device command. The host process uses `Command::new(&self.adb_path).args(&cmd_args)` at `src/tools/android/adb.rs:59-62`, but the user-controlled value is intentionally supplied as the device-shell command argument at `src/tools/android/adb.rs:75-78`.
- Q4: Is the user input effectively sanitized? → NO. The blocklist only checks literal substrings `"rm -rf"` and `"rm -r"` at `src/tools/android/actions.rs:413-416`, so equivalent forms like `rm -fr` and `rm -f -r` do not match; the metacharacter check at `src/tools/android/actions.rs:434-437` does not cover option permutations.
- Q5: Is the code in test/demo/dead/generated context? → NO. The Android tool is feature-gated but production-registered under `#[cfg(feature = "android")]` at `src/cli/common.rs:884-890`, and the feature is defined in `Cargo.toml:181-202`.
- → Reached leaf: TRUE_POSITIVE

## 3. Verdict
**TRUE_POSITIVE**

**Key Evidence:**
- `src/tools/android/actions.rs:413-426` uses literal `contains()` blocklist patterns (`"rm -rf"`, `"rm -r"`) rather than parsing the command and options.
- `src/tools/android/actions.rs:434-445` only blocks a small metacharacter set before executing the original `cmd` with `adb.shell(cmd).await`.
- `src/tools/android/mod.rs:255-260` passes the tool argument `command` directly into `actions::device_shell()`.
- `src/security/agent_mode.rs:16-30` and `src/security/agent_mode.rs:89-93` show the default agent mode is `Autonomous` and allows the `Hardware` category by default.

## 4. Remediation Suggestions
- Replace the literal substring blocklist with a strict allowlist of permitted Android shell subcommands, or remove the arbitrary `shell` action and expose only specific safe actions.
- If `rm` must be supported, parse shell tokens and canonicalize short-option clusters (`-fr`, `-rf`, `-f -r`) before policy checks; reject recursive/force deletion regardless of option order.
- Treat `android` as dangerous in approval defaults and require explicit approval for `action: "shell"`, not only for the top-level host `shell` tool.
- After fixing, add tests for `rm -fr`, `rm -f -r`, `rm --recursive --force`, and whitespace/case variations in `device_shell()`.
