T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:34
- Finding
- Command Injection Through Unsafe Interpolation of User-Controlled Location Data<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 34-38 **Vulnerability Type**: OS command injection caused by unsafe shell-command construction **Risk Level**: High ### Vulnerable Code ```markdown ### Fetching Prayer Times by City ```bash curl -L "https://api.aladhan.com/v1/timingsByCity?city={CITY}&country={COUNTRY}&method={METHOD}" ``` Replace `{CITY}`, `{COUNTRY}`, and `{METHOD}` with actual values. URL-encode spaces (e.g., `New%20York`). Always use `-L` to follow redirects. ``` ### Technical Analysis The Skill instructs the Agent to substitute location values into a shell command. City and country values originate from user input, but the instructions only require spaces to be URL-encoded. They do not require comprehensive URL encoding, shell-safe argument handling, character validation, or rejection of shell metacharacters. Wrapping the URL in double quotes does not neutralize shell command substitution. Constructs such as `$(command)` and backticks remain active inside double-quoted shell strings. An input containing a double quote can also terminate the quoted URL and introduce additional shell syntax. For example, if a malicious city value is interpolated directly: ```text $(id) ``` the generated command could become: ```bash curl -L "https://api.aladhan.com/v1/timingsByCity?city=$(id)&country=Example&method=3" ``` The shell evaluates `id` before starting `curl`. A more harmful payload could invoke other locally available commands. The vulnerability arises from generating executable shell text from untrusted data rather than passing data as independently encoded process arguments. The network access itself is necessary for the declared prayer-time functionality and is limited in the documentation to the Aladhan API. However, exposing untrusted input to a general-purpose shell exceeds the minimum execution capability required for an HTTP lookup. ### Attack Path 1. An attacker asks for prayer times using a crafted city ...[truncated 1593 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Avoid shell command construction.** Use a structured HTTP client or Agent HTTP tool that accepts the endpoint and query parameters separately. 2. **If `curl` is required, pass arguments without `eval` or generated shell text.** Use variables and `--get --data-urlencode`, for example: ```bash city="$1" country="$2" method="$3" case "$method" in 1|2|3|4|5|7|8|9|10|11|12|13|14|15) ;; *) printf '%s\n' "Invalid calculation method" >&2; exit 2 ;; esac curl --fail --silent --show-error \ --proto '=https' \ --proto-redir '=https' \ --max-redirs 3 \ --get 'https://api.aladhan.com/v1/timingsByCity' \ --data-urlencode "city=$city" \ --data-urlencode "country=$country" \ --data-urlencode "method=$method" ``` User input stored in a shell variable is not recursively interpreted as shell syntax when the variable is expanded normally. The implementation must not use `eval`, `sh -c` with concatenated input, or equivalent dynamic execution. 3. **Validate structured fields.** - Restrict `method` to the documented numeric allowlist. - Require dates to match the expected format and verify that they are valid dates. - Parse latitude and longitude as finite decimal numbers and enforce ranges of `-90` to `90` and `-180` to `180`, respectively. - Reject NUL bytes, newlines, and other control characters in city and country values. 4. **Encode all query parameters.** Do not limit encoding to spaces. Apply standards-compliant percent encoding to every user-controlled query value. 5. **Reduce execution privileges.** Prefer a domain-restricted HTTP capability over general shell access. If shell execution cannot be eliminated, run it in a sandbox with minimal filesystem, environment, process, and network access. 6. **Clarify the Skill instructions.** Explicitly state that location values are untrusted and must never be directly interpolated into executable shell command strings. ]]>