T09 · Insecure Skill Coding Practices
Warning
- Location
SKILL.md:101- Finding
Unvalidated Subject Allows Conditional Output Path Traversal
- Content
View full analysis
1 && echo "More content..." || echo ""))\n\n## Summary\n- Sections: $sections\n- Length: ~$wordcount words" polishedPath="$outputDir/${ts}-${subject}.md" echo -e "$polished_content" > "$polishedPath" save_state polishedPath subject ``` ### Technical Analysis The automatically inferred `subject` is converted into a restricted slug, but a caller-supplied `input_subject` bypasses that sanitization entirely. It may therefore contain path separators, `..` components, control characters, or other unsafe filename characters. The unvalidated value is interpolated into `polishedPath` and used as the destination of a shell redirection. Quoting the path prevents shell command injection, but it does not prevent filesystem path traversal. The timestamp is prepended to the subject, which prevents a simple value such as `../../target` from immediately becoming a parent-directory traversal: its first path component becomes something similar to `2609161200-..`. Exploitation is nevertheless possible when the attacker can prepare a matching intermediate directory or symbolic link under the output directory. Timestamps are predictable to the minute, making such preparation feasible where the attacker has access to the output directory. ### Attack Path 1. The attacker can control `input_subject` and can create files, directories, or symbolic links inside the selected output directory. 2. The a ...[truncated 1787 chars]- Remediation
View remediation
&2 exit 1 fi ``` 2. Explicitly reject path separators, traversal components, and control characters before constructing the output path: ```sh case "$subject" in *"/"*|*".."*|*"\\"*) echo "Invalid subject" >&2 exit 1 ;; esac ``` 3. Enforce a clear allowlist, such as: ```text ^[a-z0-9][a-z0-9-]{0,63}$ ``` 4. Canonicalize `outputDir` and the destination parent, then verify that the resolved destination remains beneath the canonical output directory. This check should also account for symbolic links. 5. Reject a destination if it already exists as a symbolic link. Where supported, use file-creation mechanisms that do not follow symlinks and consider refusing to overwrite existing files. 6. Replace `echo -e` with `printf` to avoid implementation-dependent escape processing: ```sh printf '%s\n' "$polished_content" > "$polishedPath" ``` 7. Add tests covering subjects containing `/`, `\`, `..`, absolute-looking paths, newlines, Unicode separators, oversized values, and prepared symbolic links. ]]>