T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/release-check.sh:43- Finding
Predictable Temporary Files Allow Symlink-Based File Clobbering
- Content
View full analysis
/tmp/release-guard-shell-eval.$$ 2>/dev/null; then warn "Potential dynamic shell evaluation found; review these lines:" sed -n '1,20p' /tmp/release-guard-shell-eval.$$ else pass "No dynamic shell evaluation found outside release-check.sh." fi rm -f /tmp/release-guard-shell-eval.$$ if grep -RInE '(api[_-]?key|secret|token|password)[[:space:]]*[:=]' "$skill_dir" \ --exclude-dir=.git --exclude-dir=node_modules >/tmp/release-guard-secret.$$ 2>/dev/null; then warn "Secret-like assignment text found; manually verify it is not a real credential:" sed -n '1,20p' /tmp/release-guard-secret.$$ else ``` ### Technical Analysis The script constructs temporary paths in the shared `/tmp` directory using only the shell process ID (`$$`). Process IDs are observable or predictable by other local users. The files are then opened using ordinary shell output redirection without atomic exclusive creation, ownership validation, or protection against symbolic links. If an attacker creates one of these paths as a symbolic link before the corresponding redirection occurs, the shell follows the link and opens its target with truncation enabled. The target is consequently truncated and then populated with the output produced by `grep`. The later `rm -f` operation removes the attacker-created symbolic link rather than reversing modifications made to its target. This behavior also conflicts with the documented claim that the skill does not modify files. ### Attack Path 1. A local attacker observes or predicts the process ID of a user running `release-check.sh`. 2. The attacker creates a symbolic link wit ...[truncated 1386 chars]- Remediation
View remediation
&2 exit 1 } trap 'rm -rf -- "$tmp_dir"' EXIT HUP INT TERM shell_eval_output="$tmp_dir/shell-eval" secret_output="$tmp_dir/secrets" ``` Use `$shell_eval_output` and `$secret_output` for the two redirections. Quote every reference to these paths: ```bash grep ... >"$shell_eval_output" sed -n '1,20p' "$shell_eval_output" grep ... >"$secret_output" sed -n '1,20p' "$secret_output" ``` Additional hardening measures include: - Do not create predictable files directly under a shared temporary directory. - Fail safely if `mktemp` cannot create the private directory. - Retain `umask 077` so temporary scan results are not readable by other users. - Register cleanup immediately after successful directory creation so files are removed on normal exit and common signals. - Avoid running the release checker with elevated privileges because its checks do not require them. ]]>