T09 · Insecure Skill Coding Practices
Error
- Location
SKILL.md:31- Finding
Shell Command Injection Through Unquoted Configuration Placeholders
- Content
View full analysis
/tmp/reddit_fetch_{SUBREDDIT}.log 2>&1 ``` ### Technical Analysis The skill instructs the agent to interpolate `BASE_DIR`, `DATE`, and `SUBREDDIT` into shell commands. These values are configurable through command-line arguments or environment variables, but the command templates do not consistently pass them as quoted shell arguments. The first command interpolates all three values without quoting: ```bash mkdir -p {BASE_DIR}/{DATE}/{SUBREDDIT}/temp ``` In the second command, `{SUBREDDIT}` is unquoted when supplied to `autocli`, and it is also interpolated into an unquoted redirection path: ```bash autocli reddit subreddit {SUBREDDIT} ... > /tmp/reddit_fetch_{SUBREDDIT}.log ``` If an agent substitutes attacker-controlled configuration text into these templates and executes the result through a shell, shell metacharacters such as semicolons, command substitutions, pipes, or redirections can be interpreted as syntax rather than data. Although `scripts/fetch_post_details.py` invokes `autocli` using a subprocess argument array and does not use `shell=True`, that protection does not cover the shell commands prescribed by `SKILL.md`. ### Attack Path 1. An attacker causes a crafted value to be used as `SUBREDDIT` or `BASE_DIR`, either through a user request, a command-line configuration value, or a relevant environment variable. 2. For example, a malicious subreddit value could contain shell syntax resembling: ```text Cl ...[truncated 1426 chars]- Remediation
View remediation
&2 exit 1 fi ``` 2. **Validate the date independently.** Require the expected `YYYYMMDD` representation: ```bash if [[ ! "$DATE" =~ ^[0-9]{8}$ ]]; then printf 'Invalid date\n' >&2 exit 1 fi ``` 3. **Quote every variable expansion used as a shell argument or path:** ```bash TEMP_DIR="${BASE_DIR}/${DATE}/${SUBREDDIT}/temp" mkdir -p -- "$TEMP_DIR" ``` 4. **Avoid inserting the subreddit into a log filename directly.** Construct the path only after validation and quote the redirection target: ```bash LOG_FILE="/tmp/reddit_fetch_${SUBREDDIT}.log" autocli reddit subreddit "$SUBREDDIT" \ --limit 20 \ --sort top \ --time day \ --format json | python3 "$SKILL_DIR/scripts/fetch_post_details.py" \ --temp-dir "$TEMP_DIR" >"$LOG_FILE" 2>&1 ``` 5. **Prefer a wrapper implemented with argument arrays.** Move orchestration into Python and invoke external commands using `subprocess` with a list of arguments and `shell=False`. This avoids requiring the agent to assemble a shell command from user-controlled text. 6. **Constrain the output directory.** Resolve the configured base path to a canonical path and, if the product permits it, require it to remain under an approved output root. Reject unexpected traversal or sensitive system locations. 7. **Document that configuration values must never be concatenated into executable shell text.** Treat values obtained from user requests, command-line parameters, and environment variables as untrusted input. ]]>