T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:434
- Finding
- Shell Command Injection Through Unescaped Prompt and Failure-Log Interpolation<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md:434-461` (additional affected pattern at `SKILL.md:487-507`) **Vulnerability Type**: Shell command injection caused by constructing interactive shell commands from untrusted text **Risk Level**: High ### Vulnerable Code ```bash FAILURE_LOG=$(tail -500 "$LOG_FILE") ERROR_LINES=$(grep -n -i "error\|fail\|panic\|exception\|traceback" "$LOG_FILE" | tail -50) if [ -n "$ERROR_LINES" ]; then FAILURE_LOG="=== Error lines === $ERROR_LINES === Last 500 lines === $FAILURE_LOG" fi ``` The captured output is subsequently inserted directly into a command sent to an interactive shell: ```bash tmux send-keys -t "$TASK_ID" "claude --dangerously-skip-permissions \ 'Previous attempt failed. Error output: $FAILURE_LOG CI status: $CI_LOG Fix the issues above and complete the original task. [...your enriched instructions here...] When done: commit, push, gh pr create --fill, then run: openclaw system event --text \"Done: $TASK_ID (retry $RETRY)\" --mode now'" Enter ``` A similar unsafe command-construction pattern is used by the parallel-execution helper: ```bash launch_agent() { local TASK_ID="$1" WORKTREE="$2" PROMPT="$3" local LOG_FILE="$WORKTREE/claude-output.log" tmux new-session -d -s "$TASK_ID" -c "$WORKTREE" tmux pipe-pane -t "$TASK_ID" -o "sed 's/\x1b\[[0-9;]*[a-zA-Z]//g' >> $LOG_FILE" tmux send-keys -t "$TASK_ID" "$PROMPT" Enter } ``` ### Technical Analysis `tmux send-keys` does not pass an argument array directly to a process. It types the supplied text into the shell running in the target pane. Consequently, the shell parses all quotes, newlines, substitutions, redirections, separators, and other metacharacters contained in that text. The retry workflow places `FAILURE_LOG` and `CI_LOG` inside a single-quoted Claude prompt embedded in a larger shell command. These values can contain repository-controlled build output, test failures, filenames, exception messages, or other attacker- ...[truncated 2382 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not construct shell commands by interpolating prompts, logs, CI output, task identifiers, or paths into text sent through `tmux send-keys`. 2. Pass prompt data through a securely created file or standard input. For example: - Create a temporary file with `mktemp`. - Set permissions to `0600`. - Write prompt content with `printf '%s'`. - Invoke a fixed wrapper script that reads the file as data. - Delete the file after execution. 3. Prefer direct process execution with an argument array instead of passing a command through an interactive shell. 4. If shell construction is unavoidable, escape every dynamic argument using a robust mechanism such as Bash `printf '%q'`. Do not attempt ad hoc quote replacement. 5. Validate identifiers such as `TASK_ID` and branch names against strict allowlists, for example `^[A-Za-z0-9._-]+$`. 6. Treat repository output, test output, CI output, filenames, and previous model responses as untrusted input. 7. Remove `--dangerously-skip-permissions` from retry commands so a second security boundary remains if malicious content reaches the agent. 8. Add adversarial tests containing single quotes, double quotes, newlines, command substitutions, semicolons, pipes, and redirections in prompts and captured logs. ]]>