T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/prepare_mint.sh:59
- Finding
- Shell Command Injection Through Unsafe Arithmetic Evaluation<![CDATA[ ## Vulnerability Details **File Location**: `scripts/prepare_mint.sh:59-63`; `scripts/config.sh:67-92`; `scripts/check_price.sh:27-35`; `scripts/find_available_plots.sh:38-50` **Vulnerability Type**: Command injection through unvalidated Bash arithmetic expressions **Risk Level**: High ### Vulnerable Code ```bash # scripts/prepare_mint.sh:59-63 WIDTH=$((X2 - X1)) HEIGHT=$((Y2 - Y1)) # Step 1: Validate coordinates echo "Validating coordinates..." >&2 validate_coords "$X1" "$Y1" "$X2" "$Y2" || exit 1 ``` ```bash # scripts/config.sh:67-92 validate_grid_aligned() { local val="$1" local name="$2" if (( val % GRID_UNIT != 0 )); then echo "Error: $name ($val) must be a multiple of $GRID_UNIT" >&2 return 1 fi if (( val < 0 || val > CANVAS_SIZE )); then echo "Error: $name ($val) must be between 0 and $CANVAS_SIZE" >&2 return 1 fi return 0 } validate_coords() { local x1="$1" y1="$2" x2="$3" y2="$4" validate_grid_aligned "$x1" "x1" || return 1 validate_grid_aligned "$y1" "y1" || return 1 validate_grid_aligned "$x2" "x2" || return 1 validate_grid_aligned "$y2" "y2" || return 1 if (( x2 <= x1 )); then echo "Error: x2 ($x2) must be greater than x1 ($x1)" >&2 return 1 fi if (( y2 <= y1 )); then echo "Error: y2 ($y2) must be greater than y1 ($y1)" >&2 return 1 fi return 0 } ``` ```bash # scripts/check_price.sh:27-35 else X1="$1" Y1="$2" X2="$3" Y2="$4" WIDTH=$((X2 - X1)) HEIGHT=$((Y2 - Y1)) fi ``` ```bash # scripts/find_available_plots.sh:38-50 if (( WIDTH % GRID_UNIT != 0 )); then echo "Error: Width ($WIDTH) must be a multiple of $GRID_UNIT" >&2 exit 1 fi if (( HEIGHT % GRID_UNIT != 0 )); then echo "Error: Height ($HEIGHT) must be a multiple of $GRID_UNIT" >&2 exit 1 fi if (( WIDTH < GRID_UNIT || HEIGHT < GRID_UNIT )); then echo "Error: Minimum size is ${GRID_UNIT}x${GRID_UNIT}" >&2 ...[truncated 2531 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate every numeric argument before it appears in any arithmetic context: ```bash validate_decimal_integer() { local value="$1" local name="$2" if [[ ! "$value" =~ ^[0-9]+$ ]]; then printf 'Error: %s must be an unsigned decimal integer\n' "$name" >&2 return 1 fi } ``` 2. Invoke lexical validation immediately after parsing arguments and before calculating width, height, modulo, comparisons, or ranges: ```bash validate_decimal_integer "$X1" x1 || exit 1 validate_decimal_integer "$Y1" y1 || exit 1 validate_decimal_integer "$X2" x2 || exit 1 validate_decimal_integer "$Y2" y2 || exit 1 WIDTH=$((10#$X2 - 10#$X1)) HEIGHT=$((10#$Y2 - 10#$Y1)) ``` 3. Use the `10#` prefix after validation to force base-10 interpretation and avoid leading-zero octal behavior. 4. Apply the same validation to `WIDTH`, `HEIGHT`, `LIMIT`, and every other value later used in an arithmetic expression. 5. Reject missing values after options such as `--limit` rather than evaluating an absent or unrelated argument. 6. Add regression tests containing command substitutions, array syntax, operators, whitespace, signs, hexadecimal notation, leading zeros, newlines, and nonnumeric characters. Tests should verify that rejection occurs before any arithmetic evaluation or side effect. ]]>