T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/compress_novel_memory.sh:19
- Finding
- Command Injection Through Unvalidated Arithmetic Argument<![CDATA[ ## Vulnerability Details **File Location**: `scripts/compress_novel_memory.sh`, lines 19–46; additional arithmetic use at lines 75, 109, 113, 130, and 134 **Vulnerability Type**: Bash arithmetic-expression command injection **Risk Level**: High ### Vulnerable Code ```bash # Parse arguments MEMORY_FILE="" while [[ $# -gt 0 ]]; do case $1 in --keep-chapters) KEEP_CHAPTERS="$2" shift 2 ;; *) MEMORY_FILE="$1" shift ;; esac done # Check file if [[ -z "$MEMORY_FILE" ]] || [[ ! -f "$MEMORY_FILE" ]]; then echo "Usage: $0 <memory_file.md> [--keep-chapters N]" exit 1 fi # Get file information ORIGINAL_SIZE=$(wc -c < "$MEMORY_FILE") TITLE=$(grep -oP '(?<=# 《)[^》]+' "$MEMORY_FILE" 2>/dev/null || echo "Unknown novel") TOTAL_CHAPTERS=$(grep -c "^### 第" "$MEMORY_FILE" 2>/dev/null || echo "0") # The unvalidated value is evaluated as a Bash arithmetic expression. if [[ "$TOTAL_CHAPTERS" -le "$KEEP_CHAPTERS" ]]; then echo " Chapter count does not exceed the threshold; no compression is needed" exit 0 fi ``` The same untrusted value is subsequently used in further arithmetic contexts: ```bash CHAPTER_START=$((TOTAL_CHAPTERS - KEEP_CHAPTERS + 1)) ``` ### Technical Analysis The `--keep-chapters` argument is assigned directly to `KEEP_CHAPTERS` without validating that it is a positive decimal integer. Bash numeric comparisons and arithmetic expansions do not treat variable contents as inert strings. Instead, values can be recursively interpreted as arithmetic expressions. Arithmetic expressions can reference array subscripts, and those subscripts may contain command substitutions. Consequently, a crafted argument such as an expression structurally equivalent to: ```bash 'x[$(ATTACKER_COMMAND)0]' ``` can cause `ATTACKER_COMMAND` to run when Bash evaluates `KEEP_CHAPTERS` in the `-le` comparison or the later `$((...))` expression. Quoting the variable i ...[truncated 1766 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Validate the option before it reaches any numeric comparison or arithmetic expansion: ```bash while [[ $# -gt 0 ]]; do case "$1" in --keep-chapters) if [[ $# -lt 2 ]] || [[ ! "$2" =~ ^[1-9][0-9]*$ ]]; then printf '%s\n' "Error: --keep-chapters requires a positive decimal integer." >&2 exit 2 fi KEEP_CHAPTERS=$2 shift 2 ;; --) shift break ;; -*) printf 'Error: unknown option: %s\n' "$1" >&2 exit 2 ;; *) if [[ -n "$MEMORY_FILE" ]]; then printf '%s\n' "Error: multiple memory files were supplied." >&2 exit 2 fi MEMORY_FILE=$1 shift ;; esac done ``` Additional hardening measures: 1. Enforce a reasonable upper bound, such as `1–10000`, to prevent pathological input. 2. Reject signed numbers, whitespace, arithmetic operators, variable names, brackets, and command-substitution syntax. 3. Validate `TOTAL_CHAPTERS` before using it in arithmetic, even though it currently originates from `grep -c`. 4. Use `printf` rather than `echo` for diagnostics containing variable data. 5. Add regression tests proving rejection of missing values, negative values, nonnumeric input, arithmetic expressions, array-subscript expressions, and command-substitution payloads. 6. Run the script with the least filesystem and network privileges required for compression. ]]>