T09 · Insecure Skill Coding Practices
- Location
scripts/script.sh:8- Finding
Command Injection Through Unvalidated Arithmetic Expansion
- Content
View full analysis
Vulnerability Details
File Location:
scripts/script.sh, lines 8-32
Vulnerability Type: Bash arithmetic command injection
Risk Level: HighVulnerable Code
bash cmd_calculate() { local period="${1:-14}" cat << EOF ═══════════════════════════════════════════════════ RSI Calculator — Period: ${period} ═══════════════════════════════════════════════════ 【RSI Formula】 RSI = 100 - (100 / (1 + RS)) RS = Average Gain / Average Loss (over ${period} periods) 【Step-by-step Calculation】 1. Collect ${period} periods of closing prices 2. Calculate price changes: Change = Close[i] - Close[i-1] 3. Separate gains (positive changes) and losses (negative changes) 4. Average Gain = Sum of gains / ${period} 5. Average Loss = Sum of losses / ${period} 6. RS = Average Gain / Average Loss 7. RSI = 100 - (100 / (1 + RS)) 【Smoothed RSI (Wilder's method, more common)】 After first ${period} periods: Avg Gain = (Prev Avg Gain × $(( period - 1 )) + Current Gain) / ${period} Avg Loss = (Prev Avg Loss × $(( period - 1 )) + Current Loss) / ${period}Technical Analysis
The
calculatecommand copies its first command-line argument into theperiodvariable without validating that it is an integer. The variable is subsequently referenced by Bash arithmetic expansions:bash $(( period - 1 ))Bash recursively evaluates the value of a variable used in an arithmetic expression. A malicious value can therefore contain an arithmetic array-subscript expression with command substitution. When Bash evaluates
period, the command substitution embedded in that expression is executed by the shell.Quoting the original assignment does not prevent this issue because the dangerous evaluation occurs later, inside the arithmetic expansion. The use of
set -euo pipefailalso does not prevent command substitution from running; the script may terminate after the payload has already executed.For example, a local ca ...[truncated 2093 chars]
- Remediation
View remediation
Remediation Suggestions
Validate the argument before it is used in any arithmetic context. Accept only ASCII decimal digits and enforce a reasonable positive range:
bash cmd_calculate() { local period="${1:-14}" if [[ ! "$period" =~ ^[0-9]+$ ]]; then printf 'Error: period must be a positive integer.\n' >&2 return 1 fi if (( 10#$period < 1 || 10#$period > 1000 )); then printf 'Error: period must be between 1 and 1000.\n' >&2 return 1 fi local period_num=$((10#$period)) local previous_period=$((period_num - 1))Use only the validated numeric variables in output:
bash Avg Gain = (Prev Avg Gain × ${previous_period} + Current Gain) / ${period_num} Avg Loss = (Prev Avg Loss × ${previous_period} + Current Loss) / ${period_num}Additional hardening measures:
- Reject signs, whitespace, variable names, brackets, command substitutions, and other non-digit characters.
- Use
10#after validation to force base-10 interpretation and avoid unintended octal handling of leading zeros. - Enforce an upper bound to prevent unreasonable or misleading period values.
- Add regression tests using malicious arithmetic expressions and verify that no external command executes.
- Avoid evaluating untrusted strings through Bash arithmetic expansion anywhere else in the Skill.
- If arithmetic is not necessary for the displayed reference text, remove the dynamic arithmetic expansion entirely.