T09 · Insecure Skill Coding Practices
Warning
- Location
- SKILL.md:16
- Finding
- Potential Shell Command Injection Through Dynamically Constructed URL Parameters<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 16–17 and line 74 **Vulnerability Type**: Shell command injection **Risk Level**: Medium ### Vulnerable Code Snippet ```markdown Run via `nodes.run` on the Mac node using `bash -c "open 'pomoclaw://...'"`: ``` ```markdown - Use `bash -c "open 'pomoclaw://...'"` via nodes.run (more reliable than array format with URL encoding) ``` The documented command patterns include dynamically supplied values, such as: ```text pomoclaw://start?minutes=N pomoclaw://break?minutes=N pomoclaw://config?sound=Glass ``` ### Technical Analysis The Skill explicitly directs the Agent to construct a URL containing request-derived parameters and pass it through `bash -c`. A shell is unnecessary for opening the URL and introduces an additional parsing layer. The URL is enclosed in single quotes, but the instructions do not require the Agent to validate every parameter according to an allowlist or escape values for shell use. If attacker-controlled text containing a single quote and shell operators is substituted into a URL parameter, it can terminate the quoted URL and add a second shell command. For example, unsafe interpolation of a crafted value conceptually resembling: ```text 25'; attacker_command; # ``` could produce a command structurally equivalent to: ```bash bash -c "open 'pomoclaw://start?minutes=25'; attacker_command; #'" ``` The shell would interpret `attacker_command` as a separate command rather than as part of the URL. The documented `1–99` timer range reduces the expected input domain, but the Skill does not explicitly instruct the Agent to parse the value as an integer and reject all nonnumeric input before constructing the shell command. Configuration values similarly require strict validation. ### Attack Path 1. An attacker submits a timer or configuration request containing a quote and shell metacharacters in a parameter value. 2. The Agent follows the Skill and interpolates tha ...[truncated 1213 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Remove `bash -c` from the invocation path.** Invoke `/usr/bin/open` directly through an argument-array execution API so that the URL is passed as one literal argument and is never parsed as shell syntax. For example, use the conceptual equivalent of: ```text executable: /usr/bin/open arguments: - pomoclaw://start?minutes=25 ``` 2. **Strictly validate numeric parameters.** - Parse timer durations as integers rather than strings. - Accept only values from `1` through `99`. - Reject signs, whitespace, decimal notation, shell metacharacters, and trailing text. - Apply documented bounds to configuration durations and completed-count values. 3. **Allowlist enumerated parameters.** - Accept sound names only from the documented sound list. - Accept Boolean settings only as exact canonical values such as `true` or `false`. - Reject unknown configuration keys. 4. **Encode URL components.** Construct the custom URL with a URL-component encoder after semantic validation. URL encoding is defense in depth and should not be treated as a substitute for avoiding shell execution. 5. **Add an explicit safety rule to the Skill.** State that raw user text must never be interpolated into a shell command and that malformed values must produce an error rather than being passed through. 6. **Add adversarial validation tests.** Test values containing quotes, semicolons, command substitutions, newlines, redirection operators, and URL delimiters to verify that they are rejected before any node command is issued. ]]>