T09 · Insecure Skill Coding Practices
Warning
- Location
- SKILL.md:51
- Finding
- Predictable Temporary File Allows Symlink-Based File Corruption<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 51–65 **Vulnerability Type**: Predictable and insecure temporary-file handling **Risk Level**: Medium ### Vulnerable Code ```bash for f in "/path/to/directory/"*.txt; do perl -ne ' next if $. == 1; s/^\x{FEFF}//; if (/^(\S+)\s+(\d{2}):(\d{2})/) { $speaker = $1; $mm = $2; $ss = $3; $hh = int($mm / 60); $mm = $mm % 60; s/^(\S+)\s+(\d{2}):(\d{2})/sprintf("[%02d:%02d:%02d]%s", $hh, $mm, $ss, $speaker)/e; } print; ' "$f" > "${f}.tmp" && mv "${f}.tmp" "$f" done ``` ### Technical Analysis The batch-processing command uses the predictable path `${f}.tmp` as a temporary output file. Shell output redirection opens this path before executing Perl and follows symbolic links by default. The code neither creates the temporary file securely nor verifies that the path is a regular file owned by the invoking user. If an attacker can write to the transcript directory, the attacker can create `${f}.tmp` as a symbolic link to another file writable by the user running the conversion. The redirection then truncates that target and writes converted transcript content into it. After successful processing, `mv "${f}.tmp" "$f"` can also replace the original transcript path with the attacker-created symbolic link. The vulnerability is constrained by operating-system permissions: it does not grant privileges beyond those of the user invoking the command. It nevertheless creates an avoidable local symlink race and unintended file-overwrite primitive. ### Attack Path 1. An attacker identifies a transcript that will be processed, such as `interview.txt`. 2. The attacker has write access to the transcript directory and creates `interview.txt.tmp` as a symbolic link to another file writable by the victim user. 3. The victim or Agent runs the documented batch conversion. 4. The shell evaluates `> ...[truncated 843 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Generate temporary files unpredictably with `mktemp`, placing them in the same directory as the destination so the final rename remains atomic. - Refuse to use an existing temporary path and do not follow symbolic links. - Register a cleanup trap so temporary files are removed after errors or interruption. - Verify that each input is a regular file and that the generated temporary path differs from the input. - Rename the temporary file over the source only after Perl completes successfully. - Restrict processing directories so untrusted users cannot create or replace entries within them. A hardened pattern is: ```bash for f in "/path/to/directory/"*.txt; do [ -f "$f" ] || continue dir=$(dirname -- "$f") base=$(basename -- "$f") tmp=$(mktemp -- "$dir/.${base}.tmp.XXXXXX") || exit 1 trap 'rm -f -- "$tmp"' EXIT HUP INT TERM if perl -ne ' next if $. == 1; s/^\x{FEFF}//; if (/^(\S+)\s+(\d{2}):(\d{2})/) { $speaker = $1; $mm = $2; $ss = $3; $hh = int($mm / 60); $mm = $mm % 60; s/^(\S+)\s+(\d{2}):(\d{2})/sprintf("[%02d:%02d:%02d]%s", $hh, $mm, $ss, $speaker)/e; } print; ' -- "$f" > "$tmp"; then mv -- "$tmp" "$f" trap - EXIT HUP INT TERM else rm -f -- "$tmp" trap - EXIT HUP INT TERM exit 1 fi done ``` ]]>