T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/script.sh:26
- Finding
- Arbitrary Command Execution Through Unvalidated Bash Arithmetic Expressions<![CDATA[ ## Vulnerability Details **File Location**: `scripts/script.sh`, lines 26–66, 81–98, 127–158, and 264–298 **Vulnerability Type**: Command injection through unsafe arithmetic evaluation **Risk Level**: High ### Vulnerable Code ```bash --weeks) weeks="$2"; shift 2 ;; ``` ```bash echo " \"total_hours\": $(( weeks * hours_per_week ))," for (( w=1; w<=weeks; w++ )); do local comma="," [[ ${w} -eq ${weeks} ]] && comma="" echo " {\"week\": ${w}, \"focus\": \"Week ${w} — ${topic} block ${w}\", \"hours\": ${hours_per_week}}${comma}" done ``` ```bash --count) count="$2"; shift 2 ;; ``` ```bash for (( i=1; i<=count; i++ )); do echo "Q${i}. [${qtype}] [${difficulty}] Question about ${topic} — concept ${i}" done ``` ```bash --hours-per-day) hours_per_day="$2"; shift 2 ;; --days) days="$2"; shift 2 ;; ``` ```bash echo "Start: ${start_date} | ${days} days | ${hours_per_day}h/day | Total: $(( days * hours_per_day ))h" ``` ```bash echo "Total study time: $(( days * hours_per_day )) hours over ${days} days." ``` ### Technical Analysis The `--weeks`, `--count`, `--days`, and `--hours-per-day` arguments are accepted as arbitrary strings and later used in Bash arithmetic expansions or arithmetic `for` loops. Bash arithmetic evaluation does not merely convert these strings to integers. Variable values used in arithmetic contexts can be interpreted recursively as arithmetic expressions. Crafted expressions can include array-subscript syntax containing command substitutions. When Bash evaluates such an expression, the command substitution can execute an arbitrary local command. Quoting the variable when it is initially assigned does not prevent this issue because the dangerous interpretation occurs later, when the variable is consumed by `$((...))` or `((...))`. The affected inputs and evaluation points include: - `--weeks`: arithmetic multiplication and loop conditions. - `--count`: loop conditions in quiz and flashcard generation. - `--d ...[truncated 2332 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Validate every numeric option immediately after argument parsing and before using it in any Bash arithmetic context. Use strict decimal-integer checks and reasonable upper bounds: ```bash validate_positive_integer() { local name="$1" local value="$2" local maximum="$3" if [[ ! "$value" =~ ^[0-9]+$ ]] || (( 10#$value < 1 || 10#$value > maximum )); then echo "Invalid ${name}: expected an integer from 1 to ${maximum}" >&2 return 1 fi } ``` Apply validation to all affected options: ```bash validate_positive_integer "weeks" "$weeks" 520 || return 1 validate_positive_integer "count" "$count" 1000 || return 1 validate_positive_integer "days" "$days" 3660 || return 1 validate_positive_integer "hours-per-day" "$hours_per_day" 24 || return 1 ``` Additional hardening measures: 1. Use the `10#` prefix only after regex validation to force decimal interpretation and avoid unintended octal handling. 2. Reject zero, negative values, signs, whitespace, arithmetic operators, variable names, brackets, and command-substitution syntax. 3. Add upper bounds to prevent excessive CPU consumption or output generation from extremely large loop counts. 4. Check that every option requiring a value has a following argument before reading `$2`. 5. Consider moving numeric calculations and iteration into Python after strict conversion with `int()` and explicit range checks. 6. Add regression tests asserting that payloads containing `$()`, backticks, brackets, operators, variable names, negative values, and oversized integers are rejected without side effects. ]]>