T09 · Insecure Skill Coding Practices
Error
- Location
- etf-assistant.sh:232
- Finding
- Arbitrary Command Execution Through Bash Arithmetic Injection<![CDATA[ ## Vulnerability Details **File Location**: `etf-assistant.sh`, lines 232–250 **Vulnerability Type**: Bash arithmetic-expression injection **Risk Level**: High ### Vulnerable Code ```bash cmd_calc() { local code=$1 local amount=$2 local years=$3 if [ -z "$code" ] || [ -z "$amount" ] || [ -z "$years" ]; then echo -e "${RED}❌ 参数不全${NC}" echo "示例: $0 calc 510300 1000 10" echo "含义: 每月定投1000元，定投10年" return 1 fi local name=$(get_etf_name "$code") echo -e "${GREEN}📈 定投计算器${NC}" echo "━━━━━━━━━━━━━━━━━━━━" echo "ETF: $name ($code)" echo "月定投: ¥$amount" echo "定投年限: $years 年" echo "━━━━━━━━━━━━━━━━━━━━" echo "" # 简化计算 (假设年化收益率8%) local months=$((years * 12)) local annual_return=0.08 local monthly_return=$(echo "scale=6; $annual_return / 12" | bc) # 使用复利公式计算 local future_value=$(echo "scale=2; $amount * ((1 + $monthly_return)^$months - 1) / $monthly_return" | bc) local total_invest=$((amount * months)) ``` ### Technical Analysis The `amount` and `years` values come directly from command-line arguments and are checked only for emptiness. They are not validated as decimal integers before being evaluated by Bash arithmetic expansion. Bash arithmetic contexts do more than convert strings to numbers. Their operands are parsed as arithmetic expressions, and variable references can be recursively evaluated. Constructs such as array subscripts may trigger expansions, including command substitution, during arithmetic evaluation. The following statements are therefore dangerous sinks: ```bash local months=$((years * 12)) local total_invest=$((amount * months)) ``` An attacker-controlled arithmetic expression placed in `years` can be evaluated when `months` is assigned. A malicious expression in `amount` can similarly be evaluated when `total_invest` is assigned. The intermediate use of `amount` in the command sent to `bc` also al ...[truncated 1748 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Validate all calculator operands before they enter Bash arithmetic expansion or `bc`. Accept only a narrowly defined decimal representation and impose reasonable bounds: ```bash if ! [[ "$amount" =~ ^[1-9][0-9]*$ ]]; then echo "Amount must be a positive integer." >&2 return 1 fi if ! [[ "$years" =~ ^[1-9][0-9]*$ ]]; then echo "Years must be a positive integer." >&2 return 1 fi if (( amount > 100000000 || years > 100 )); then echo "Amount or duration exceeds the supported range." >&2 return 1 fi ``` After validation, force base-10 interpretation to prevent leading-zero values from being treated as octal: ```bash local amount_num=$((10#$amount)) local years_num=$((10#$years)) local months=$((years_num * 12)) local total_invest=$((amount_num * months)) ``` Construct the `bc` input exclusively from these validated numeric variables: ```bash local future_value future_value=$(printf '%s\n' \ "scale=2; $amount_num * ((1 + $monthly_return)^$months - 1) / $monthly_return" | bc) ``` Additional hardening should include: - Reject signs, whitespace, decimal points, variable names, brackets, parentheses, and shell metacharacters unless explicitly required. - Check for integer overflow before multiplication. - Use `local variable; variable=$(...)` rather than combining declaration and command substitution, so command failures are not masked by `local`. - Handle `bc` failures explicitly and return a nonzero status. - Run the Skill with a restricted account and minimal filesystem, environment, and network access. ]]>