T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/build_showcase.sh:10
- Finding
- FFmpeg Argument Injection Through Unquoted Input Construction<![CDATA[ ## Vulnerability Details **File Location**: `scripts/build_showcase.sh`, lines 10–32 **Vulnerability Type**: Shell word splitting and FFmpeg argument injection **Risk Level**: Medium ### Vulnerable Code ```bash FFMPEG="${FFMPEG:-ffmpeg}" # Build segments from all PNGs in order i=0 FILTER="" INPUTS="" for img in "$IMAGES_DIR"/*.png; do [ -f "$img" ] || continue INPUTS="$INPUTS -loop 1 -t $DURATION -i $img" if [ $i -eq 0 ]; then FILTER="[$i:v]scale=${RES}:force_original_aspect_ratio=decrease,pad=${RES}:(ow-iw)/2:(oh-ih)/2,setsar=1,fade=t=out:st=$((DURATION-1)):d=1[v$i];" else FILTER="$FILTER [$i:v]scale=${RES}:force_original_aspect_ratio=decrease,pad=${RES}:(ow-iw)/2:(oh-ih)/2,setsar=1,fade=t=in:st=0:d=1,fade=t=out:st=$((DURATION-1)):d=1[v$i];" fi i=$((i+1)) done # Concat CONCAT="" for j in $(seq 0 $((i-1))); do CONCAT="${CONCAT}[v$j]"; done $FFMPEG $INPUTS -filter_complex "$FILTER ${CONCAT}concat=n=$i:v=1:a=0[out]" \ -map "[out]" -c:v libx264 -pix_fmt yuv420p -r $FPS "$OUTPUT" ``` ### Technical Analysis The script constructs all FFmpeg input arguments in the scalar string `INPUTS`. Each image path is inserted into that string without preserving its argument boundary: ```bash INPUTS="$INPUTS -loop 1 -t $DURATION -i $img" ``` The string is subsequently expanded without quotes: ```bash $FFMPEG $INPUTS ... ``` Bash therefore performs word splitting and pathname expansion on the complete value. An image filename containing spaces is not passed to FFmpeg as one path. Instead, filename components are interpreted as independent command-line arguments. Components beginning with `-` can consequently be interpreted as FFmpeg options rather than as part of the filename. The executable selector is also expanded as unquoted `$FFMPEG`. Although environment control already permits selecting another executable, unquoted expansion additionally allows the value to be split into an executable and attacker-supplied ar ...[truncated 2611 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Use Bash arrays to preserve each command-line argument exactly and quote every expansion: ```bash #!/bin/bash set -euo pipefail IMAGES_DIR="${1:-./screenshots}" OUTPUT="${2:-showcase.mp4}" DURATION=4 FPS=30 RES="1920x1080" FFMPEG="${FFMPEG:-ffmpeg}" inputs=() i=0 filter="" for img in "$IMAGES_DIR"/*.png; do [[ -f "$img" ]] || continue inputs+=(-loop 1 -t "$DURATION" -i "$img") if ((i == 0)); then filter+="[$i:v]scale=${RES}:force_original_aspect_ratio=decrease," filter+="pad=${RES}:(ow-iw)/2:(oh-ih)/2,setsar=1," filter+="fade=t=out:st=$((DURATION-1)):d=1[v$i];" else filter+="[$i:v]scale=${RES}:force_original_aspect_ratio=decrease," filter+="pad=${RES}:(ow-iw)/2:(oh-ih)/2,setsar=1," filter+="fade=t=in:st=0:d=1," filter+="fade=t=out:st=$((DURATION-1)):d=1[v$i];" fi ((i += 1)) done if ((i == 0)); then printf 'Error: no PNG images found in %s\n' "$IMAGES_DIR" >&2 exit 1 fi concat="" for ((j = 0; j < i; j++)); do concat+="[v$j]" done "$FFMPEG" "${inputs[@]}" \ -filter_complex "${filter} ${concat}concat=n=$i:v=1:a=0[out]" \ -map "[out]" -c:v libx264 -pix_fmt yuv420p -r "$FPS" -- "$OUTPUT" ``` Additional hardening measures: 1. Resolve `FFMPEG` to an approved executable path or reject values containing whitespace instead of accepting an arbitrary command-like string. 2. Treat screenshot directories and filenames as untrusted input. 3. Run FFmpeg in a sandbox with minimal filesystem permissions and no network access when processing untrusted media. 4. Restrict FFmpeg protocols where supported, allowing only those required for local image processing. 5. Validate that each discovered input is a regular local file and an expected image type before invoking FFmpeg. 6. Fail explicitly when no PNG files are found rather than generating a malformed filter graph. 7. Add regression tests using filenames containing spaces, wildcard chara ...[truncated 102 chars]