T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/generate_image.sh:97
- Finding
- Arithmetic Expression Injection Through Unvalidated --max-wait Argument<![CDATA[ ## Vulnerability Details **File Location**: `scripts/generate_image.sh`, lines 97-100 and 204-210 **Vulnerability Type**: Shell arithmetic expression injection **Risk Level**: High ### Vulnerable Code ```bash --max-wait) MAX_WAIT="$2" shift 2 ;; ``` The unvalidated value is subsequently used in a Bash arithmetic expression: ```bash now_ts="$(date +%s)" elapsed="$((now_ts - start_ts))" if (( elapsed > 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
- <![CDATA[ ## Remediation Suggestions Validate `MAX_WAIT` before it enters any arithmetic context. Require a bounded, non-negative decimal integer: ```bash if [[ ! "$MAX_WAIT" =~ ^[0-9]+$ ]]; then echo "Error: invalid --max-wait (must be a non-negative integer)." >&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. ]]>