T09 · Insecure Skill Coding Practices
Error
- Location
scripts/generate_image.sh:97- Finding
Arithmetic Expression Injection Through Unvalidated --max-wait Argument
- Content
View full analysis
MAX_WAIT )); then echo "Error: timed out after ${MAX_WAIT}s." >&2 exit 1 fi ``` ### Technical Analysis The `--max-wait` value is copied directly into `MAX_WAIT` without confirming that it is a decimal integer. Bash recursively interprets variable values used in arithmetic contexts as arithmetic expressions rather than treating them strictly as numeric data. An attacker who can influence the script arguments can therefore provide a crafted arithmetic expression. Bash arithmetic expressions support constructs such as array subscripts, and command substitutions embedded in such constructs may be evaluated by the shell. Consequently, evaluation of the condition: ```bash (( elapsed > MAX_WAIT )) ``` can cause attacker-controlled shell commands to execute. The nearby `--poll-interval` argument is also not validated. It is passed to `sleep`, which can permit malformed values or excessively long delays, although it is not directly used in a Bash arithmetic expression. ### Attack Path 1. An attacker gains the ability to control or influence arguments passed to `generate_image.sh`. 2. The attacker supplies a crafted arithmetic expression through `--max-wait`. 3. The script accepts the value without numeric validation. 4. After creating an image task, the polling loop reaches the arithmetic comparison at line 207. 5. Bash recursively evaluates the attacker-controlled value as an arithmetic expression. 6. Any command substitution embedded in an evaluated arithmetic construct runs with the privileges and environment of the Skill process. ### Imp ...[truncated 572 chars]- Remediation
View remediation
&2 exit 1 fi if (( 10#$MAX_WAIT > 3600 )); then echo "Error: --max-wait must not exceed 3600 seconds." >&2 exit 1 fi ``` The `10#` prefix forces decimal interpretation after validation and avoids unintended octal handling of values with leading zeroes. Apply similar validation and reasonable bounds to `POLL_INTERVAL`: ```bash if [[ ! "$POLL_INTERVAL" =~ ^[0-9]+([.][0-9]+)?$ ]]; then echo "Error: invalid --poll-interval." >&2 exit 1 fi ``` For stronger hardening: - Reject missing option values before accessing `$2`. - Enforce minimum and maximum values for all numeric parameters. - Validate `N` against the API's documented range rather than checking only that it contains digits. - Keep attacker-controlled strings out of shell arithmetic expressions whenever possible. ]]>