T09 · Insecure Skill Coding Practices
Error
- Location
scripts/script.sh:97- Finding
Arbitrary Command Execution Through Unsafe Bash Arithmetic Expansion
- Content
View full analysis
" echo 'Meditation timer: ${2:-10} minutes'; echo 'Starting...'; sleep $((${2:-10}*60)) 2>/dev/null && echo 'Session complete' } ``` ### Technical Analysis The timer argument is inserted directly into a Bash arithmetic expansion: ```bash $((${2:-10}*60)) ``` Bash arithmetic expressions do not accept only integer literals. They can resolve variable and array expressions, and crafted array subscripts can trigger command substitution during evaluation. Because the value is not validated as a bounded decimal integer before arithmetic evaluation, an attacker-controlled argument can cause arbitrary shell commands to execute. There is also an argument-indexing error. The dispatcher shifts the command name before calling `cmd_timer`: ```bash timer) shift; cmd_timer "$@" ;; ``` After this shift, the documented `timer ` value is available as `$1`, but `cmd_timer` reads `$2`. Consequently, an ordinary invocation does not work as intended, while a crafted additional argument can reach the vulnerable arithmetic expression. ### Attack Path 1. An attacker supplies arguments to the documented `scripts/script.sh timer` command. 2. `main` removes the `timer` command name with `shift`. 3. `cmd_timer` incorrectly reads its second argument rather than its first. 4. The attacker places a crafted Bash arithmetic expression in that second argument, such as an array expression containing a command substitution. 5. The value is interpolated into `$((${2:-10}*60))`. 6. Bash evaluates the command substitution while parsing the arithmetic expression. 7. The injected command runs with the same operating-system identity, environment, filesystem access, and privileges as the Skill process. ## ...[truncated 662 chars]- Remediation
View remediation
= 1 && 10#$minutes <= 1440 )) || die "Minutes must be between 1 and 1440" ``` 4. Perform arithmetic only after validation, explicitly treating the input as base 10: ```bash local seconds=$((10#$minutes * 60)) sleep "$seconds" ``` 5. Keep expansions quoted when passed to commands and avoid placing unvalidated external input directly inside arithmetic, conditional, or indirect variable expressions. A hardened implementation would be: ```bash cmd_timer() { local minutes="${1:-}" [ -z "$minutes" ] && die "Usage: $SCRIPT_NAME timer " [[ "$minutes" =~ ^[0-9]+$ ]] || die "Minutes must be a positive integer" (( 10#$minutes >= 1 && 10#$minutes <= 1440 )) || die "Minutes must be between 1 and 1440" local seconds=$((10#$minutes * 60)) echo "Meditation timer: $minutes minutes" echo "Starting..." sleep "$seconds" echo "Session complete" } ``` Add regression tests covering valid integers, missing arguments, negative values, very large values, nonnumeric input, arithmetic syntax, array syntax, and command-substitution payloads. ]]>