T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:165
- Finding
- Shell Command Injection Through Unsafely Interpolated User Input<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 47, 165–173, and 228–232 **Vulnerability Type**: Shell command injection **Risk Level**: High ### Vulnerable Code ```bash # Line 47 wctl dispatch <session-id> "<user's message>" ``` ```markdown <!-- Lines 165–173 --> | "reject", "no", feedback like "change X to Y" | **Two steps:** `wctl dispatch <id> "4"` then wait 2s then `wctl dispatch <id> "<their feedback>"` | **Important for option 4 (feedback/reject):** This is a two-step process. First dispatch "4" to select the text input option, wait 2 seconds for the text prompt to appear, then dispatch the feedback text. Example: ```bash wctl dispatch abc123 "4" sleep 2 wctl dispatch abc123 "don't modify the database schema" ``` ``` ```bash # Lines 228–232 wctl dispatch-answers <id> --chat-arrows 20 sleep 3 wctl dispatch <id> "<user's original message>" ``` ### Technical Analysis The skill directs the agent to interpolate user-controlled text directly into shell command strings. Placing the text between double quotes does not make this safe: shell metacharacters that terminate the quoted argument can alter command structure, while command substitution such as `$(...)` and backticks remains active inside double quotes. Consequently, the shell may interpret malicious input before `wctl` receives it. The same vulnerable pattern appears in normal message dispatch, rejection feedback, and the “Chat about this” fallback. The issue is especially significant because the skill explicitly instructs the agent to pass the user's original message or feedback through these templates. The statement later in `SKILL.md` that free text is never injected into PTY commands without validation does not provide an actual escaping or validation mechanism and is inconsistent with the documented command templates. ### Attack Path 1. An attacker provides a message or rejection response containing shell syntax, such as a closing quote followed by a command separ ...[truncated 1604 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Do not assemble shell command strings from user-controlled data.** Invoke `wctl` through a structured process API with a separate argument array, for example: ```javascript spawn("wctl", ["dispatch", sessionId, userMessage], { shell: false }); ``` 2. **Disable shell interpretation explicitly.** Ensure the execution API does not use `shell: true`, `bash -c`, `sh -c`, `eval`, or equivalent command-string evaluation. 3. **Validate session identifiers separately.** Restrict session IDs to the exact format returned by `wctl list`, or require an allow-listed value from parsed command output. Do not accept an arbitrary user-supplied identifier. 4. **Treat messages as opaque data.** User messages, rejection feedback, and fallback text should be passed as one argument without modification or evaluation. Do not attempt to secure these values merely by adding double quotes. 5. **If a shell is unavoidable, use positional parameters.** Pass untrusted values as positional arguments to a fixed script rather than interpolating them into its source: ```bash sh -c 'exec wctl dispatch "$1" "$2"' sh "$session_id" "$user_message" ``` A structured argument API remains preferable. 6. **Apply the same hardening to other dynamic commands.** The `imagePath`, `channel`, and `target` values used by `openclaw message send` should also be passed through a structured argument interface and validated. 7. **Correct the security documentation.** Remove or revise the claim that free text is never injected without validation until an enforceable implementation prevents shell interpretation. 8. **Add adversarial tests.** Verify that messages containing quotes, semicolons, newlines, backticks, command substitutions, redirects, and shell operators are delivered literally to `wctl` and never executed by a shell. ]]>