T09 · Insecure Skill Coding Practices
Error
- Location
scripts/funnel.sh:53- Finding
Arbitrary Command Execution Through Unvalidated Bash Arithmetic Expressions
- Content
View full analysis
0 )); then bar_width=$((val * max_width / first_val)) fi ``` The same pattern appears in the diagnostic and optimization functions: ```bash local val="${pair##*:}" ... local conv_rate=$((val * 100 / prev_val)) local drop=$((prev_val - val)) local drop_rate=$((drop * 100 / prev_val)) ``` ```bash local val="${pair##*:}" if (( idx > 0 )); then local drop_rate=$(( (prev_val - val) * 100 / prev_val )) ``` Comparison values are also evaluated without validation: ```bash local vA="${valsA[$i]}" local vB="${valsB[$i]}" if (( i == 0 )); then printf "| 步骤%d | %s | %s | - | - |\n" "$((i+1))" "$vA" "$vB" else local rateA=$((vA * 100 / prevA)) local rateB=$((vB * 100 / prevB)) ``` ### Technical Analysis Bash arithmetic contexts such as `$((...))` and `((...))` do not merely convert strings to integers. Variable values can be recursively interpreted as arithmetic expressions. In particular, crafted array-subscript expressions can contain command substitutions that Bash executes while resolving the arithmetic expression. The script extracts count values directly from command-line input and passes them into arithmetic contexts without first requiring a canonical integer representation. Quoting the original command-line argument does not prevent this secondary evaluation inside Bash arithmetic syntax. All commands that process supplied funnel counts are affected: - ...[truncated 1428 chars]- Remediation
View remediation
&2 return 1 fi if (( 10#$value > 1000000000 )); then printf 'Count exceeds the supported limit: %s\n' "$value" >&2 return 1 fi } ``` Apply validation to every parsed value: ```bash local val="${pair##*:}" validate_count "$val" || return 1 val=$((10#$val)) ``` Use `10#` only after the regular-expression check to force base-10 interpretation and avoid octal handling of leading zeroes. Apply the same validation independently to every member of `valsA` and `valsB`. Additional hardening should include: 1. Reject empty values, signs, whitespace, variable names, array syntax, and arithmetic operators. 2. Impose a reasonable upper bound to prevent integer overflow and excessive output generation. 3. Centralize parsing in one function rather than duplicating unsafe parsing in each command. 4. Add regression tests using command substitutions, array subscripts, malformed counts, negative numbers, and oversized integers. 5. Ensure invalid input produces a controlled error before any arithmetic expression is evaluated. ]]>