T09 · Insecure Skill Coding Practices
- Location
scripts/sync-to-agents.sh:60- Finding
Unvalidated skill name permits out-of-root deletion and overwrite
- Content
View full analysis
Vulnerability Details
File Location:
scripts/sync-to-agents.sh, lines 60-61 and 105-129
Vulnerability Type: Path traversal leading to arbitrary deletion or overwrite within the current user's permissions
Risk Level: HighVulnerable Code
bash --skill-name) SKILL_NAME="$2" shift 2 ;;bash dest="$root/$SKILL_NAME" if [[ "$dest" == "$SOURCE_DIR" ]]; then echo "[$agent] source already at target ($dest), skipping" continue fi mkdir -p "$root" if [[ "$METHOD" == "symlink" ]]; then if [[ -e "$dest" || -L "$dest" ]]; then rm -rf "$dest" fi ln -s "$SOURCE_DIR" "$dest" echo "[$agent] symlinked $dest -> $SOURCE_DIR" else if [[ -L "$dest" ]]; then rm -f "$dest" fi mkdir -p "$dest" rsync -a --delete \ --exclude '.git' \ --exclude '.git/*' \ --exclude '.DS_Store' \ "$SOURCE_DIR/" "$dest/" echo "[$agent] copied to $dest" fiTechnical Analysis
The script accepts
--skill-nameas an unrestricted string and appends it directly to an agent-specific root directory. It does not reject path separators,..components, control characters, or names that resolve outside the intended skill directory.The resulting path is passed to security-sensitive filesystem operations:
rm -rf "$dest"recursively deletes an existing destination in symlink mode.mkdir -p "$dest"creates attacker-selected directory paths.rsync -a --deleteoverwrites the destination and removes files that do not exist in the source tree.ln -screates a link at the attacker-selected location.
Shell quoting prevents word splitting and command substitution, but it does not prevent filesystem path traversal. The equality check against
SOURCE_DIRprotects only one exact path and does not establish that the destination remains beneath the selected agent root.Attack Path
- An attacker influences arguments pas ...[truncated 1475 chars]
- Remediation
View remediation
Remediation Suggestions
- Treat
--skill-namestrictly as a directory basename. Permit only a narrow character set, such as letters, digits, periods, underscores, and hyphens. - Explicitly reject empty names,
.,.., slash characters, backslashes, control characters, and names beginning with traversal components. - Canonicalize the agent root and candidate destination before performing any destructive operation.
- Verify that the canonical destination is a direct child of the canonical agent root, rather than merely sharing a textual prefix.
- Add a second boundary check immediately before every
rm,rsync, andlnoperation. - Prefer deleting only a known skill directory instead of using unrestricted
rm -rf. - Consider requiring confirmation before replacing an existing non-symlink directory.
- Add regression tests using empty names,
.and.., nested paths, absolute-looking values, repeated traversal components, and destinations involving symlinks. Every test should assert that no path outside the agent skill root was modified.
- Treat