T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/setup.sh:132
- Finding
- Command Execution Through Unvalidated Bash Arithmetic Expressions<![CDATA[ ## Vulnerability Details **File Location**: `scripts/setup.sh:132-145` **Vulnerability Type**: Bash arithmetic-expression injection **Risk Level**: High ### Vulnerable Code ```bash if [[ "${USE_DEFAULT_WEIGHTS,,}" == "y" ]]; then W_EVIDENCE=35 W_ENGAGEMENT=25 W_HONESTY=20 W_PERSUASION=20 else echo " Enter weights (must sum to 100):" read -rp " Evidence & Reasoning [35]: " W_EVIDENCE W_EVIDENCE="${W_EVIDENCE:-35}" read -rp " Engagement [25]: " W_ENGAGEMENT W_ENGAGEMENT="${W_ENGAGEMENT:-25}" read -rp " Intellectual Honesty [20]: " W_HONESTY W_HONESTY="${W_HONESTY:-20}" read -rp " Persuasiveness [20]: " W_PERSUASION W_PERSUASION="${W_PERSUASION:-20}" TOTAL=$((W_EVIDENCE + W_ENGAGEMENT + W_HONESTY + W_PERSUASION)) if [[ "$TOTAL" -ne 100 ]]; then echo "Error: Weights sum to $TOTAL, must be 100." >&2 exit 1 fi fi ``` ### Technical Analysis The four weight values are read as unrestricted strings and then evaluated inside a Bash arithmetic expansion. Bash arithmetic contexts do not merely parse decimal integers: they evaluate arithmetic expressions and recursively resolve variable and array references. Malicious expressions involving array subscripts or nested substitutions can consequently trigger unintended shell evaluation. The check that the final total equals 100 occurs only after the arithmetic expression has been evaluated. It therefore cannot prevent side effects produced during evaluation. ### Attack Path 1. An attacker convinces a privileged operator or automation process to run `scripts/setup.sh`. 2. The operator chooses custom judging weights. 3. The attacker supplies a crafted Bash arithmetic expression instead of a decimal weight. 4. Line 142 evaluates the expression while calculating `TOTAL`. 5. Embedded side effects execute with the privileges of the process running the setup script. 6. The final sum validation occurs only after the malicious expressi ...[truncated 381 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Validate every weight before using it in an arithmetic context: ```bash validate_weight() { local value="$1" if [[ ! "$value" =~ ^[0-9]+$ ]]; then echo "Error: weights must be decimal integers." >&2 exit 1 fi if (( 10#$value < 0 || 10#$value > 100 )); then echo "Error: weights must be between 0 and 100." >&2 exit 1 fi } ``` Call this function for all four values, then convert them explicitly: ```bash validate_weight "$W_EVIDENCE" validate_weight "$W_ENGAGEMENT" validate_weight "$W_HONESTY" validate_weight "$W_PERSUASION" W_EVIDENCE=$((10#$W_EVIDENCE)) W_ENGAGEMENT=$((10#$W_ENGAGEMENT)) W_HONESTY=$((10#$W_HONESTY)) W_PERSUASION=$((10#$W_PERSUASION)) TOTAL=$((W_EVIDENCE + W_ENGAGEMENT + W_HONESTY + W_PERSUASION)) ``` Reject expressions, signs, whitespace, variable names, array syntax, and all other non-decimal input. ]]>