T09 · Insecure Skill Coding Practices
Warning
- Location
- SKILL.md:42
- Finding
- Predictable Temporary File Enables Symlink-Based File Modification## Vulnerability Details **File Location**: `SKILL.md`, lines 42 and 49–50; cleanup occurs at line 72 **Vulnerability Type**: Predictable and insecure temporary-file handling **Risk Level**: Medium **Vulnerable code:** ```bash TARGET="${1:-http://localhost:18789/}" COUNT="${2:-50}" TMP="/tmp/ratelimit-test-$$.txt" echo "Target: $TARGET" echo "Requests: $COUNT" echo "" for i in $(seq 1 $COUNT); do curl -s -o /dev/null -w "%{http_code}" "$TARGET" >> "$TMP" echo "" >> "$TMP" done ``` The temporary path is subsequently removed as follows: ```bash rm -f "$TMP" ``` ### Technical Analysis The script constructs a temporary-file path in the shared `/tmp` directory using only the process ID: ```bash TMP="/tmp/ratelimit-test-$$.txt" ``` Process IDs are predictable and do not provide secure filename uniqueness. The script neither creates the file atomically nor verifies that the path is a regular file owned by the current process. Shell append redirection follows symbolic links. A local attacker who can anticipate the process ID may create the expected path as a symbolic link to another file. When the script executes the append redirections, HTTP status data and newline characters are written to the symbolic link's target using the privileges of the user running the skill. The final `rm -f` removes the temporary pathname rather than undoing modifications made to the linked target. It also does not prevent exploitation during the earlier file-open operations. ### Attack Path 1. A local attacker observes current process IDs and predicts the PID likely to be assigned to the validator process. 2. The attacker creates `/tmp/ratelimit-test-<predicted-pid>.txt` as a symbolic link to a target file. 3. A victim invokes the rate-limit validation script with the predicted PID. 4. The shell opens the predictable path with append redirection and follows the symbolic link. 5. The script repeatedly ...[truncated 1128 chars]
- Remediation
- ## Remediation Suggestions Replace the predictable pathname with an atomically created temporary file, apply restrictive permissions, and guarantee cleanup through a trap: ```bash umask 077 TMP="$(mktemp "${TMPDIR:-/tmp}/ratelimit-test.XXXXXX")" || { echo "Failed to create temporary file" >&2 exit 1 } trap 'rm -f -- "$TMP"' EXIT ``` Additional hardening measures should include: 1. Do not run the validator with elevated privileges. 2. Use `mktemp` rather than PID-derived names or manual existence checks. 3. Quote the temporary path consistently and use `--` when passing it to utilities. 4. Retain the cleanup trap so the file is removed on normal exit, interruption, and most error paths. 5. Optionally verify that `${TMPDIR:-/tmp}` refers to an appropriate trusted temporary directory before creating the file.