T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/script.sh:108
- Finding
- Unquoted File Arguments Allow GNU grep Option Injection and Unintended File Disclosure## Vulnerability Details **File Location**: `scripts/script.sh`, lines 108-136 **Vulnerability Type**: Unquoted shell argument and option injection **Risk Level**: Medium ### Complete Vulnerable Code ```bash cmd_lint() { local file="${2:-}" [ -z "$file" ] && die "Usage: $SCRIPT_NAME lint <file>" grep -n 'latest' $2 && echo 'WARN: Avoid :latest tag'; grep -c RUN $2 | awk '{if($1>5) print "WARN: Too many RUN layers"}' } cmd_optimize() { local file="${2:-}" [ -z "$file" ] && die "Usage: $SCRIPT_NAME optimize <file>" echo '=== Optimization suggestions for $2 ==='; grep -c RUN $2 | awk '{if($1>3) print "Combine RUN commands"}' } cmd_scan() { local file="${2:-}" [ -z "$file" ] && die "Usage: $SCRIPT_NAME scan <file>" echo 'Scanning $2...'; grep -in 'password\|secret\|key' $2 && echo 'WARN: Potential secrets' || echo 'Clean' } ``` The associated dispatch logic is: ```bash lint) shift; cmd_lint "$@" ;; optimize) shift; cmd_optimize "$@" ;; scan) shift; cmd_scan "$@" ;; ``` ### Technical Analysis The handlers use `${2:-}` and `$2` even though the dispatcher first removes the command name with `shift`. Under the documented invocation `script.sh scan <file>`, the requested filename consequently becomes `$1`, while `$2` is empty and the command fails. An attacker can provide an additional argument so that attacker-controlled content occupies `$2`. That value is then passed to `grep` without quoting and without the `--` end-of-options delimiter. This creates two related shell-safety problems: 1. Shell word splitting and pathname expansion may convert one supplied value into multiple file operands. 2. A value beginning with `-` may be interpreted as a GNU `grep` option rather than as a filename. For example, supplying `-R` as the second handler argument can enable recursive searching. Because the ...[truncated 2091 chars]
- Remediation
- ## Remediation Suggestions 1. Use `$1` after the dispatcher removes the command name: ```bash local file="${1:-}" ``` 2. Quote every filename expansion to prevent word splitting and pathname expansion. 3. Insert `--` before file operands so values beginning with `-` cannot become options: ```bash grep -n -- 'latest' "$file" grep -c -- 'RUN' "$file" grep -in -- 'password\|secret\|key' "$file" ``` 4. Validate that the argument identifies an expected regular file before reading it: ```bash [[ -f "$file" ]] || die "Not a regular file: $file" ``` 5. If scans must be restricted to a trusted project directory, canonicalize the path and verify that it remains beneath that directory. Consider rejecting symbolic links when they are not required. 6. Refactor the affected handlers consistently. For example: ```bash cmd_scan() { local file="${1:-}" [[ -n "$file" ]] || die "Usage: $SCRIPT_NAME scan <file>" [[ -f "$file" ]] || die "Not a regular file: $file" printf 'Scanning %s...\n' "$file" if grep -in -- 'password\|secret\|key' "$file"; then echo 'WARN: Potential secrets' else echo 'Clean' fi } ``` 7. Add regression tests covering ordinary filenames, filenames containing spaces or wildcard characters, filenames beginning with `-`, missing arguments, extra arguments, and symbolic links.