T09 · Insecure Skill Coding Practices
Warning
- Location
- SKILL.md:13
- Finding
- Shell Command Injection Through Unvalidated Smart-Home Targets<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md:13-17` **Supporting Location**: `references/api.md:3-5, 127-131` **Vulnerability Type**: Command injection caused by unsafe shell-command construction **Risk Level**: Medium ### Vulnerable Code Snippets `SKILL.md:13-17`: ```markdown ## Core Pattern All control and query happens via `curl http://localhost:8423/<action>/<target>`. - Targets use `Room/Device` format or just `DeviceName` - Spaces → `%20` in URLs ``` `references/api.md:3-5`: ```markdown Base URL: `http://localhost:8423` (default port, configurable in Settings) All endpoints respond with JSON. Targets are case-insensitive. Use `%20` for spaces. ``` `references/api.md:127-131`: ```markdown ## Target Format - `Room/Device` → most specific, preferred - `DeviceName` → matches across all rooms (may be ambiguous) - `RoomName` → matches all devices in room (for info/list only) - URL encode spaces: `Living Room` → `Living%20Room` ``` ### Technical Analysis The Skill directs the agent to interpolate an action and target into a shell command containing a URL. The only documented encoding requirement is replacement of spaces with `%20`. It does not require full URL path-segment encoding, validation of target characters, action allowlisting, or shell-safe argument handling. Room, device, group, and scene names may derive from user requests or local smart-home metadata. If an agent builds the documented command as a shell string, metacharacters such as semicolons, command substitutions, redirection operators, or shell control operators can be interpreted by the shell rather than treated as part of the target. Quoting a dynamically assembled URL is not a complete defense when the implementation constructs a command string and evaluates it through a shell. Command substitution and incorrect escaping can remain exploitable depending on how interpolation occurs. The safe approach is to avoid shell parsing entirely. ### Attack Path 1. ...[truncated 1568 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Do not construct shell command strings.** Invoke the HTTP client through a structured process API with a fixed executable and separate argument array, or use a native HTTP library. 2. **Allowlist actions.** Accept only explicitly supported endpoint names such as `info`, `on`, `off`, `brightness`, and `scene`. 3. **Encode each path segment independently.** Use a standard percent-encoding function rather than replacing spaces manually. Encode room, device, group, and scene names as data, preserving separators only when intentionally introduced by trusted code. 4. **Validate target input.** Reject control characters, URL delimiters, shell metacharacters, malformed percent escapes, and unexpected path traversal components. 5. **Avoid shell evaluation.** Do not use `eval`, `sh -c`, `bash -c`, interpolated command templates, or equivalent APIs. 6. **Validate numeric parameters.** Enforce the documented ranges for brightness, hue, saturation, fan speed, and blind position before constructing requests. 7. **Require explicit confirmation for high-impact operations.** Unlocking doors, opening garages, and similar physical-security actions should require clear user confirmation immediately before execution. 8. **Update the documentation with a safe implementation pattern.** For example, construct the URL using a URL builder and send it through an HTTP library, making clear that user-controlled values must never be interpolated into shell source. ]]>
