T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/insurance.sh:350
- Finding
- Arbitrary Command Execution Through Unvalidated Arithmetic Input<![CDATA[ ## Vulnerability Details **File Location**: `scripts/insurance.sh`, lines 350-377 **Vulnerability Type**: Arithmetic expression injection leading to local command execution **Risk Level**: High ### Vulnerable Code ```bash cmd_term() { local age="${1:?Usage: term <age> <coverage> <years>}" local coverage="${2:?Please provide coverage}" local years="${3:?Please provide coverage duration}" local age_factor premium if [ "$age" -lt 30 ]; then age_factor="0.8" elif [ "$age" -lt 40 ]; then age_factor="1.0" elif [ "$age" -lt 50 ]; then age_factor="1.5" else age_factor="2.5"; fi premium=$(echo "scale=0; $coverage * 0.0015 * $age_factor" | bc) local total total=$(echo "$premium * $years" | bc) cat <<EOF ... Coverage duration: ${years} years (until ${age}+${years}=$((age+years)) years old) Estimated annual premium: ¥$(printf "%'.0f" "$premium") Total payments: ¥$(printf "%'.0f" "$total") Leverage ratio: 1:$(echo "scale=0; $coverage / $total" | bc) ... EOF } ``` The displayed English labels reproduce the meaning of the original output text; the executable expressions are unchanged. ### Technical Analysis The `age`, `coverage`, and `years` arguments are accepted without validating that they contain only decimal integers. The user-controlled `years` value is subsequently referenced inside Bash arithmetic expansion: ```bash $((age+years)) ``` Bash arithmetic expressions can recursively interpret variable values as arithmetic syntax. Crafted expressions involving array subscripts and command substitutions can therefore cause shell commands to be evaluated while the arithmetic expression is resolved. Passing the value through `bc` earlier is not a security control. `bc` may report malformed input without sanitizing or replacing the original `years` variable, which is later passed directly into Bash arithmetic evaluation. The same general hardening requirement applies to all numeric arguments used in comparisons, `bc` expressi ...[truncated 1325 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Validate every numeric argument before it is used by `test`, `bc`, `printf`, or Bash arithmetic: ```bash require_positive_integer() { local name="$1" local value="$2" if [[ ! "$value" =~ ^[0-9]+$ ]] || (( 10#$value <= 0 )); then printf 'Error: %s must be a positive decimal integer.\n' "$name" >&2 exit 1 fi } require_positive_integer "age" "$age" require_positive_integer "coverage" "$coverage" require_positive_integer "years" "$years" ``` Additional hardening should include: 1. Enforce realistic upper and lower bounds for age, coverage, income, and duration. 2. Convert validated values to canonical decimal integers before arithmetic use. 3. Do not treat `bc` as a validation or sanitization layer. 4. Prefer passing fixed-format operands to `bc` only after strict allowlist validation. 5. Add regression tests containing shell metacharacters, command substitutions, array syntax, whitespace, signs, decimals, and extremely large values. 6. Reject invalid insurance types rather than silently applying a default rate. ]]>