T09 · Insecure Skill Coding Practices
Error
- Location
scripts/pre-publish-scan.sh:29- Finding
Filename word splitting allows files to bypass security scanning
- Content
View full analysis
/dev/null) else FILES="$TARGET" fi for f in $FILES; do SCANNED=$((SCANNED + 1)) CONTENT=$(cat "$f" 2>/dev/null || continue) ``` The generated pre-commit hook contains the same unsafe pattern: ```bash # scripts/install-hook.sh:42-55 STAGED=$(git diff --cached --name-only --diff-filter=ACM) if [ -z "$STAGED" ]; then exit 0 fi TMPDIR=$(mktemp -d) trap 'rm -rf "$TMPDIR"' EXIT FAIL=0 for f in $STAGED; do mkdir -p "$TMPDIR/$(dirname "$f")" git show ":$f" > "$TMPDIR/$f" 2>/dev/null || continue done ``` ### Technical Analysis The scripts serialize filename lists into scalar shell variables and subsequently iterate over them using unquoted expansions. Bash applies word splitting and pathname expansion to `$FILES` and `$STAGED`. A filename containing spaces, tabs, newlines, or shell glob characters can therefore be divided into multiple unrelated path components or expanded against files in the current directory. In the pre-commit hook, a failed `git show ":$f"` operation is silently ignored through `|| continue`, so omitted staged files do not cause the commit to fail. This defect undermines the primary security guarantee of the project: that all staged content is scanned before a commit is accepted. ### Attack Path 1. An attacker or repository contributor creates a file whose name contains whitespace or another filename metacharacter. 2. The attacker places a credential, ...[truncated 1186 chars]- Remediation
View remediation
/dev/null || continue) # Perform checks here. done < <( find "$TARGET" -type f \ ! -path '*/.git/*' \ ! -path '*/node_modules/*' \ ! -name '*.png' ! -name '*.jpg' ! -name '*.gif' \ ! -name '*.ico' ! -name '*.woff*' ! -name '*.ttf' \ ! -name '*.zip' ! -name '*.tar*' ! -name '*.gz' \ -print0 2>/dev/null ) ``` For staged Git paths: ```bash while IFS= read -r -d '' f; do mkdir -p -- "$TMPDIR/$(dirname -- "$f")" if ! git show ":$f" > "$TMPDIR/$f"; then echo "Failed to extract staged file: $f" >&2 exit 1 fi done < <(git diff --cached --name-only --diff-filter=ACM -z) ``` Additional hardening should include: - Use `--` where supported to prevent paths beginning with `-` from being interpreted as options. - Fail closed when staged content cannot be extracted. - Add tests for spaces, tabs, newlines, wildcard characters, leading hyphens, and non-ASCII filenames. - Avoid storing filename lists in scalar shell variables. ]]>