T09 · Insecure Skill Coding Practices
Error
- Location
SKILL.md:45- Finding
Shell Command Injection Through an Unsafely Generated Cleanup Path
- Content
View full analysis
)` after sending. ``` ```python # Unique per-channel filename with timestamp — no topic/thread collisions safe_ch = channel.replace("/", "_").replace(" ", "_") ts = int(time.time() * 1000) out_base = os.path.join(out_dir, f"gif_{safe_ch}_{ts}") ``` ```python gif_path = os.path.join(out_dir, f"source_{channel}.gif") urllib.request.urlretrieve(result["gif_url"], gif_path) conv = convert(gif_path, channel, out_dir) ``` ### Technical Analysis The `--channel` value contributes directly to generated filenames. The only sanitization applied to the converted output filename replaces forward slashes and spaces: ```python safe_ch = channel.replace("/", "_").replace(" ", "_") ``` This does not remove or reject shell metacharacters such as semicolons, command substitutions, backticks, quotes, redirection operators, or newline characters. The source GIF filename uses the channel value without even this limited sanitization. The script returns the resulting path to the Agent, while `SKILL.md` instructs the Agent to delete that path using the shell-oriented operation `exec(rm )`. If the Agent interpolates the returned path into this command without robust argument separation or shell escaping, shell metacharacters embedded in the channel value can be interpreted as executable syntax. The use of `subprocess.run()` for FFmpeg does not itself introduce shell injection because it passes an argument list and does not enable `shell=True`. The vulnerable boundary is the documented cleanup operation performed after the script returns an attacker-influenced path. ### Attack Path 1. An attacker causes a crafted channel identifier to be passed to the script, directly th ...[truncated 1386 chars]- Remediation
View remediation
`. 4. Validate channel identifiers against a strict allowlist derived from `CHANNEL_PROFILES` or discovered channel IDs. 5. Generate cache filenames independently of user-controlled values, such as with `tempfile`, a UUID, or a cryptographically random token. 6. If a channel label must appear in a filename, permit only a narrow character set such as ASCII letters, digits, underscores, and hyphens. Reject rather than partially transform invalid identifiers. 7. Ensure the final resolved file path remains inside the expected cache directory before creating, returning, or deleting it. 8. Update `SKILL.md` to prescribe a non-shell cleanup mechanism. ]]>