T09 · Insecure Skill Coding Practices
Error
- Location
scripts/divine.sh:7- Finding
Command Injection Through Unvalidated Dice Argument
- Content
View full analysis
&2; exit 1 fi local result=$(($(rand $n) + 1)) echo "🎲 Würfel (1-${n}): ${result}" } ``` ### Technical Analysis The `dice` function accepts its first argument from the command line and uses it directly in Bash arithmetic contexts: ```bash [[ $n -lt 1 ]] ``` The value is subsequently passed to `rand`, assigned to `max`, and evaluated again: ```bash $(( ... % max )) ``` Bash arithmetic operands are expressions rather than strictly parsed integers. Arithmetic evaluation can recursively resolve variable names and array subscripts. A malicious expression containing an array reference with a command substitution in its subscript can therefore cause Bash to execute that substitution while evaluating the expression. The minimum-value check does not provide input validation because it is itself an arithmetic evaluation sink. The payload may consequently be evaluated before the script decides whether the requested number of dice sides is valid. ### Attack Path 1. An attacker influences the arguments used when the Agent invokes the documented `dice` operation. 2. The attacker supplies an arithmetic expression instead of a decimal integer as the second command-line argument: ```bash bash scripts/divine.sh dice '' ``` 3. `divine.sh` assigns the expression to `n` without validating its syntax. 4. Bash evaluates the attacker-controlled value in `[[ $n -lt 1 ]]`. 5. If the expression contains a command substitution through a recursively evaluated arithmetic construct, that command runs with the privi ...[truncated 1114 chars]- Remediation
View remediation
&2 return 1 fi if (( n < 1 || n > 1000000 )); then echo "Dice size must be between 1 and 1000000." >&2 return 1 fi local result result=$(( $(rand "$n") + 1 )) printf '🎲 Dice (1-%d): %d\n' "$n" "$result" } ``` Harden `rand` independently so it remains safe if called from another function: ```bash rand() { local max=${1-} if [[ ! $max =~ ^[0-9]+$ ]] || (( max < 1 || max > 1000000 )); then echo "Invalid random-number bound." >&2 return 1 fi local value value=$(od -An -tu4 -N4 /dev/urandom | tr -d ' ') printf '%d\n' "$(( value % max ))" } ``` Additional hardening measures: 1. Enforce a reasonable maximum to prevent overflow and unexpected resource use. 2. Use `return 1` inside functions rather than terminating the entire calling process with `exit`. 3. Add regression tests covering alphabetic input, signs, whitespace, arithmetic operators, array syntax, command-substitution syntax, zero, and excessively large values. 4. Treat all values entering Bash arithmetic expansion as untrusted until they pass strict lexical validation. ]]>