T09 · Insecure Skill Coding Practices
Error
- Location
- handler.js:12
- Finding
- Host-Shell Command Injection Through Unsafe tmux Command Construction## Vulnerability Details **File Location**: `handler.js`, lines 12–14 **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```js function sendCommand(cmd) { const escaped = cmd.replace(/"/g, '\\"'); execSync(`tmux send-keys -t claw "${escaped}" C-m`); } ``` ### Technical Analysis The `cmd` value is supplied by the caller and interpolated into a command string executed by `execSync()`. Node.js passes this string to a host shell. The implementation escapes only double-quote characters, which does not prevent shell evaluation of command substitutions such as `$()` or backticks. Consequently, a crafted value can cause the host shell to execute commands while constructing the arguments for `tmux`. This execution occurs outside the designated tmux session and therefore violates the documented isolation expectation that commands are run inside session `claw`. The `isDangerous()` substring denylist does not eliminate this vulnerability. It only detects a small set of literal strings and does not account for shell substitutions, alternate command syntax, variable expansion, encoding, or equivalent utilities. A payload containing a substitution that does not include one of the blocked substrings can reach `sendCommand()` and execute in the host shell. ### Attack Path 1. An attacker or untrusted caller invokes `claw_shell_run` with a crafted `command` containing shell substitution syntax, such as: ```sh echo $(touch /tmp/claw-shell-injection-proof) ``` 2. The command passes the denylist because it contains none of the listed blocked substrings. 3. `sendCommand()` escapes only double quotes and interpolates the remaining input into: ```sh tmux send-keys -t claw "echo $(touch /tmp/claw-shell-injection-proof)" C-m ``` 4. The host shell evaluates `$(touch /tmp/claw-shell-injection-proof)` before `tmux` receives its arguments. 5. The injected command executes direc ...[truncated 968 chars]
- Remediation
- ## Remediation Suggestions Do not construct shell command strings with caller-controlled input. Invoke `tmux` directly with an argument array and disable shell interpretation. Send the command as literal text and submit Enter separately, for example: ```js const { execFileSync } = require("node:child_process"); function sendCommand(cmd) { execFileSync("tmux", ["send-keys", "-t", "claw", "-l", "--", cmd], { shell: false }); execFileSync("tmux", ["send-keys", "-t", "claw", "C-m"], { shell: false }); } ``` Additional hardening should include: 1. Avoid using shell-backed `execSync()` for all fixed tmux operations; use `execFileSync()` or `spawnSync()` with fixed executable names and argument arrays. 2. Treat substring denylisting as defense-in-depth only, not as an injection control. 3. Implement explicit authorization state if dangerous commands are intended to run after user approval. The current API has no approval field and therefore cannot securely distinguish an approved request from an unapproved one. 4. Apply an allowlist or a structured command policy if the tool does not require unrestricted shell access. 5. Run the skill under a dedicated, least-privileged operating-system account with restricted filesystem, credential, and network access. 6. Add regression tests using `$()`, backticks, quotes, newlines, semicolons, pipes, redirections, and variable expansion to verify that input reaches tmux literally and is never evaluated by the host shell.