T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/sync-to-agents.sh:60
- Finding
- Unvalidated skill name permits out-of-root deletion and overwrite## 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**: High ### Vulnerable 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" fi ``` ### Technical Analysis The script accepts `--skill-name` as 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 --delete` overwrites the destination and removes files that do not exist in the source tree. - `ln -s` creates 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_DIR` protects only one exact path and does not establish that the destination remains beneath the selected agent root. ### Attack Path 1. An attacker influences arguments pas ...[truncated 1475 chars]
- Remediation
- ## Remediation Suggestions 1. Treat `--skill-name` strictly as a directory basename. Permit only a narrow character set, such as letters, digits, periods, underscores, and hyphens. 2. Explicitly reject empty names, `.`, `..`, slash characters, backslashes, control characters, and names beginning with traversal components. 3. Canonicalize the agent root and candidate destination before performing any destructive operation. 4. Verify that the canonical destination is a direct child of the canonical agent root, rather than merely sharing a textual prefix. 5. Add a second boundary check immediately before every `rm`, `rsync`, and `ln` operation. 6. Prefer deleting only a known skill directory instead of using unrestricted `rm -rf`. 7. Consider requiring confirmation before replacing an existing non-symlink directory. 8. 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.