T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/script.sh:316
- Finding
- Arbitrary Command Execution Through Bash Arithmetic Injection<![CDATA[ ## Vulnerability Details **File Location**: `scripts/script.sh:316-331` **Vulnerability Type**: Bash arithmetic injection **Risk Level**: High ### Vulnerable Code ```bash cmd_dca() { local ticker="${1:?Usage: investment-portfolio dca <TICKER> <monthly_amount>}" local monthly="${2:?Missing monthly amount}" ticker=$(echo "$ticker" | tr 'a-z' 'A-Z') echo -e "${BOLD}DCA Calculator: $ticker @ \$$monthly/month${RESET}" echo "" echo " Month Investment Cumulative" echo " ──────────────────────────────────" local total=0 for m in $(seq 1 12); do total=$((total + monthly)) printf " %-7d \$%-12s \$%s\n" "$m" "$monthly" "$total" done echo "" echo " Total invested after 12 months: \$$total" } ``` ### Technical Analysis The `monthly` parameter comes directly from a command-line argument and is not validated before being referenced in Bash arithmetic expansion: ```bash total=$((total + monthly)) ``` Bash recursively interprets the value of a variable used in an arithmetic expression as another arithmetic expression. Arithmetic syntax can include array references whose subscripts undergo shell expansion. Consequently, a malicious value containing an array-subscript expression with command substitution can cause Bash to execute a command while evaluating `monthly`. Merely quoting the argument when invoking the script does not make the arithmetic evaluation safe. The dangerous interpretation happens later, at line 327, inside Bash's arithmetic evaluator. ### Attack Path 1. An attacker supplies or persuades a user or agent to supply a crafted value as the second argument to `dca`. 2. The value is stored unchanged in the `monthly` shell variable. 3. The loop reaches `total=$((total + monthly))`. 4. Bash recursively parses the value of `monthly` as an arithmetic expression. 5. A command substitution embedded in an arithmetic array subscript is evaluated by the shell. 6. The injected comman ...[truncated 876 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Validate the argument before it reaches any arithmetic context. If only whole-dollar amounts are supported, require an unsigned integer: ```bash [[ "$monthly" =~ ^[0-9]+$ ]] || die "Monthly amount must be a non-negative integer" ``` Apply a reasonable upper bound as well to prevent integer overflow or resource-related problems: ```bash (( monthly <= 1000000000 )) || die "Monthly amount is too large" ``` If decimal currency values must be supported, avoid Bash arithmetic and parse the value using Python's `decimal.Decimal` with a strict regular expression and explicit bounds. Pass the value through an environment variable or positional argument rather than interpolating it into generated Python or shell source. Recommended hardening steps: 1. Reject signs, whitespace, array syntax, operators, substitutions, and nonnumeric characters. 2. Define whether negative investments and decimal values are valid. 3. Enforce upper and lower bounds. 4. Add regression tests using arithmetic expressions, array references, command substitutions, empty input, large integers, and decimal input. 5. Never pass untrusted text into Bash arithmetic evaluation unless it has first been reduced to a canonical numeric representation. ]]>