T09 · Insecure Skill Coding Practices
Warning
- Location
- clip.sh:7
- Finding
- Predictable Shared Temporary Directory Enables File Races and Symlink Attacks<![CDATA[ ## Vulnerability Details **File Location**: `clip.sh`, lines 7-8 and 38-53 **Vulnerability Type**: Predictable and insecure temporary-file handling **Risk Level**: Medium ### Vulnerable Code ```bash WORK_DIR="/tmp/youtube-clipper" mkdir -p "$OUT_DIR" "$WORK_DIR" ``` ```bash SRC="$WORK_DIR/${NAME}_source.mp4" OUT="$OUT_DIR/${NAME}.mp4" REAL_OUT="$(cd "$OUT_DIR" && realpath -m "$OUT")" [[ "$REAL_OUT" == "$OUT_DIR/"* ]] || { echo "Invalid clip name: escapes output directory"; exit 2; } rm -f "$SRC" echo "Downloading..." yt-dlp --no-playlist -f "bv*+ba/b" --merge-output-format mp4 -o "$SRC" -- "$URL" [[ ! -f "$SRC" ]] && { echo "Download failed"; exit 1; } echo "Clipping..." # ffmpeg -y -ss "$START" -to "$END" -i "$SRC" -c:v libx264 -preset veryfast -crf 23 -pix_fmt yuv420p -c:a aac -b:a 192k -movflags +faststart "$OUT" ffmpeg -y -ss "$START" -to "$END" -i "$SRC" -c:v libx264 -preset medium -crf 23 -pix_fmt yuv420p -c:a aac -b:a 192k -movflags +faststart "$OUT" rm -f "$SRC" ``` ### Technical Analysis The script stores downloaded media in the fixed directory `/tmp/youtube-clipper` and derives the source filename solely from a predictable, sanitized clip name. It does not create a private per-execution directory with an unpredictable name, verify ownership and permissions of an existing directory, or atomically reserve the source file. The initial `rm -f "$SRC"` does not eliminate the vulnerability because checking or removing a pathname and subsequently passing that same pathname to `yt-dlp` are separate operations. Another local process can alter the path between those operations. An attacker may also pre-create `/tmp/youtube-clipper` before the skill is first executed, causing the script to reuse an attacker-controlled directory because `mkdir -p` accepts an existing directory without validating its owner or mode. Depending on the filesystem and downloader behavior, an attacker-controlled file or symbolic link can redirect or interfere with w ...[truncated 2279 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Create a private, unpredictable temporary directory for every execution using `mktemp -d`. Restrict its permissions and install an exit trap so cleanup occurs on success, failure, interruption, or termination. ```bash WORK_DIR="$(mktemp -d "${TMPDIR:-/tmp}/youtube-clipper.XXXXXX")" chmod 700 "$WORK_DIR" cleanup() { rm -rf -- "$WORK_DIR" } trap cleanup EXIT HUP INT TERM SRC="$WORK_DIR/source.mp4" ``` Additional hardening measures: 1. Do not reuse a fixed directory under `/tmp`. 2. Fail immediately if `mktemp -d` cannot securely create the directory. 3. Keep all intermediate files inside the private per-run directory. 4. Use a constant source filename inside that unpredictable directory rather than deriving it from user input. 5. Retain quoting and `--` argument separators when invoking tools and deleting files. 6. Use the cleanup trap instead of relying only on the final `rm -f`, ensuring downloaded media is removed after errors. 7. If a shared directory must be used, validate that it is a real directory, is owned by the invoking user, is not a symbolic link, and has mode `0700`; a private `mktemp` directory is nevertheless preferable. ]]>