T09 · Insecure Skill Coding Practices
Error
- Location
- direct-control.md:20
- Finding
- Shell Command Injection Through Unsanitized Direct-Control Parameters<![CDATA[ ## Vulnerability Details **File Location**: `direct-control.md`, lines 20, 25–26, 35–39, 46–47, and 55 **Vulnerability Type**: Shell command injection **Risk Level**: High ### Vulnerable Code ```bash atvremote -n "Kitchen HomePod" device_info atvremote -n "Kitchen HomePod" playing atvremote -n "Kitchen HomePod" volume ``` ```markdown | 1 | `atvremote -n "<target>" playing` | Read-only | Verify current media state | | 2 | `atvremote -n "<target>" play_pause` | Mutating | Toggle playback quickly | | 3 | `atvremote -n "<target>" set_volume=<0-100>` | Mutating | Set exact output level | | 4 | `atvremote -n "<target>" next` | Mutating | Advance to next track | | 5 | `atvremote -n "<target>" stop` | Mutating | End active playback session | ``` ```bash atvremote -n "<target>" play_url="https://example.com/audio.mp3" atvremote -n "<target>" stream_file="/absolute/path/to/file.mp3" ``` ```bash atvremote -n "<target>" pair ``` ### Technical Analysis The direct-control runbook instructs the agent to substitute device targets, URLs, local file paths, and volume values into shell command templates. These values can originate from device discovery or user input, but the runbook does not require strict validation, shell-safe escaping, or execution through a non-shell argument-array API. Double quotes do not prevent every form of shell evaluation. In POSIX-compatible shells, command substitutions such as `$(command)` and backtick substitutions remain active inside double-quoted strings. Consequently, a malicious value inserted into `<target>`, the URL, or the file path could cause an additional local command to execute if the agent constructs and invokes the resulting string through a shell. This weakness is particularly relevant to device names returned by local discovery. An attacker capable of advertising a crafted device name on the local network could potentially introduce shell metacharacters without directly supplying a visible command to the user. Exp ...[truncated 1802 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Do not construct shell command strings.** Invoke `atvremote` through an execution API that accepts an argument array and disables shell processing. For example, pass the executable and each argument as distinct values: ```text executable: atvremote arguments: ["-n", selected_target, "playing"] shell: false ``` 2. **Validate discovered device identifiers.** Prefer immutable device identifiers or validated IP addresses over display names. If names must be accepted, reject control characters, line breaks, null bytes, and shell metacharacters. Apply explicit length limits. 3. **Validate operation-specific parameters.** - Parse URLs with a URL parser and permit only explicitly supported schemes, such as `https`. - Resolve local file paths canonically and ensure they point to an intended regular media file. - Parse volume as an integer and enforce the range `0` through `100`. - Permit commands only from a fixed allowlist such as `playing`, `play_pause`, `set_volume`, `next`, `stop`, and `pair`. 4. **Do not rely on quoting alone.** If shell invocation is unavoidable, use a well-tested, platform-specific escaping library rather than manual quoting. Account separately for POSIX shells, PowerShell, and Windows command processing. 5. **Confirm the resolved action.** Before a mutating operation, show the normalized target, operation, URL or path where applicable, and resulting effect. Do not expose or request confirmation for an unsafe raw shell command. 6. **Treat discovery results as untrusted input.** A local-network device name is attacker-influenced until validated. Do not automatically execute commands against newly discovered or ambiguous targets. 7. **Add explicit security guidance to the runbook.** State that placeholder substitution must never be performed by concatenating values into a shell command and provide safe argument-array examples for every supported platform. ]]>