T09 · Insecure Skill Coding Practices
Warning
- Location
- " grep -in 'secret\|token\|password' $2 2>/dev/null && echo 'WARN: Hardcoded secrets found' || echo 'Clean' } ``` ```bash main() { local cmd="${1:-help}" case "$cmd" in create) shift; cmd_create "$@" ;; template) shift; cmd_template "$@" ;; lint) shift; cmd_lint "$@" ;; list) shift; cmd_list "$@" ;; optimize) shift; cmd_optimize "$@" ;; secrets) shift; cmd_secrets "$@" ;; help) cmd_help ;; version) cmd_version ;; *) die "Un ...[truncated 2872 chars]:142
- Finding
- Grep Option Injection and Unintended Local File Disclosure<![CDATA[ ## Vulnerability Details **File Location**: `scripts/script.sh:142-144, 166-172` **Vulnerability Type**: Unquoted argument expansion and incorrect positional argument handling **Risk Level**: Medium ### Vulnerable Code ```bash cmd_secrets() { local file="${2:-}" [ -z "$file" ] && die "Usage: $SCRIPT_NAME secrets <file>" grep -in 'secret\|token\|password' $2 2>/dev/null && echo 'WARN: Hardcoded secrets found' || echo 'Clean' } ``` ```bash main() { local cmd="${1:-help}" case "$cmd" in create) shift; cmd_create "$@" ;; template) shift; cmd_template "$@" ;; lint) shift; cmd_lint "$@" ;; list) shift; cmd_list "$@" ;; optimize) shift; cmd_optimize "$@" ;; secrets) shift; cmd_secrets "$@" ;; help) cmd_help ;; version) cmd_version ;; *) die "Unknown: $cmd" ;; esac } ``` ### Technical Analysis The command dispatcher removes the command name with `shift`, making the documented file argument available as `$1`. However, `cmd_secrets` incorrectly reads and executes against `$2`. Consequently, a normal invocation such as `script.sh secrets workflow.yml` fails because `$2` is unset, while an invocation containing an extra argument causes the second supplied value to control `grep`. The value is also expanded without quotation and is not preceded by the `--` end-of-options delimiter: ```bash grep -in 'secret\|token\|password' $2 ``` Shell word splitting and pathname expansion can therefore turn one attacker-controlled value into multiple arguments. Values beginning with a hyphen may also be interpreted as `grep` options rather than as file names. For example, GNU `grep` can interpret `-r` as a recursive-search option, causing the command to search the current directory instead of one explicitly selected file. The same incorrect `$2` convention appears in the other parameterized handlers, including `create`, `template`, `lint`, and `optimize`. Those occurrences pri ...[truncated 1609 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Read `$1` after the dispatcher performs `shift`. 2. Use the validated local variable consistently rather than reading positional parameters again. 3. Quote every file path to prevent word splitting and pathname expansion. 4. Place `--` before user-controlled file names passed to `grep`. 5. Validate that the input identifies a regular file before scanning it. 6. Return a nonzero status for missing or invalid files instead of reporting them as clean. 7. Apply the same positional-argument and quoting corrections to `create`, `template`, `lint`, and `optimize`. A hardened implementation is: ```bash cmd_secrets() { local file="${1:-}" [ -n "$file" ] || die "Usage: $SCRIPT_NAME secrets <file>" [ -f "$file" ] || die "File not found: $file" if grep -in -- 'secret\|token\|password' "$file"; then echo 'WARN: Potential hardcoded secrets found' else echo 'Clean' fi } ``` The other handlers should likewise use `${1:-}` after dispatch: ```bash cmd_lint() { local file="${1:-}" [ -n "$file" ] || die "Usage: $SCRIPT_NAME lint <file>" [ -f "$file" ] || die "File not found: $file" grep -c -- 'runs-on' "$file" | awk '{if ($1 == 0) print "WARN: No runs-on"}' } ``` ]]>