T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/divine.sh:7
- Finding
- Command Injection Through Unvalidated Dice Argument<![CDATA[ ## Vulnerability Details **File Location**: `scripts/divine.sh`, lines 7–10 and 99–104 **Vulnerability Type**: Bash arithmetic-expression injection **Risk Level**: High ### Vulnerable Code ```bash rand() { # Zufallszahl 0 bis $1-1, via /dev/urandom local max=$1 echo $(( $(od -An -tu4 -N4 /dev/urandom | tr -d ' ') % max )) } dice() { local n=${1:-6} if [[ $n -lt 1 ]]; then echo "❌ Minimum 1 Seite!" >&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 '<crafted arithmetic expression>' ``` 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
- <![CDATA[ ## Remediation Suggestions Validate the argument as a decimal integer before placing it in any arithmetic context: ```bash dice() { local n=${1:-6} if [[ ! $n =~ ^[0-9]+$ ]]; then echo "Invalid dice size: expected a positive decimal integer." >&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. ]]>