T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/skill-sandbox.sh:108
- Finding
- Path Traversal Enables Destructive Filesystem Operations<![CDATA[ ## Vulnerability Details **File Location**: `scripts/skill-sandbox.sh`, lines 64-84, 108-109, 118-131, and 145-146 **Vulnerability Type**: Path traversal leading to arbitrary directory deletion or movement **Risk Level**: High ### Vulnerable Code ```bash while [[ $# -gt 0 ]]; do case "$1" in --force) FORCE_FLAG="--force"; shift ;; --version) VERSION_FLAG="--version $2"; shift 2 ;; --promote) PROMOTE_ONLY=true; shift ;; --scan-only) SCAN_ONLY=true; shift ;; --list-staged) LIST_STAGED=true; shift ;; --staging-dir) STAGING_DIR="$2"; shift 2 ;; --live-dir) LIVE_DIR="$2"; shift 2 ;; --help|-h) usage ;; -*) echo "Unknown option: $1"; usage ;; *) if [[ -z "$SKILL_NAME" ]]; then SKILL_NAME="$1" else echo "Unexpected argument: $1"; usage fi shift ;; esac done STAGED_PATH="$STAGING_DIR/$SKILL_NAME" LIVE_PATH="$LIVE_DIR/$SKILL_NAME" if $PROMOTE_ONLY; then if [[ ! -d "$STAGED_PATH" ]]; then echo -e "${RED}✗ Skill '$SKILL_NAME' not found in staging ($STAGING_DIR)${NC}" exit 1 fi if [[ -d "$LIVE_PATH" ]]; then echo -e "${YELLOW}⚠ Replacing existing live skill '$SKILL_NAME'${NC}" rm -rf "$LIVE_PATH" fi mv "$STAGED_PATH" "$LIVE_PATH" echo -e "${GREEN}✅ Promoted '$SKILL_NAME' → $LIVE_PATH${NC}" exit 0 fi # Clean previous staged version [[ -d "$STAGED_PATH" ]] && rm -rf "$STAGED_PATH" ``` ### Technical Analysis `SKILL_NAME` is accepted without validating that it is a single safe directory name. Values containing path separators or traversal components such as `..` are appended directly to `STAGING_DIR` and `LIVE_DIR`. Shell quoting prevents token splitting and shell metacharacter injection, but it does not prevent filesystem path traversal. The resulting paths are subsequently passed to high-impact operations including `rm -rf` and `mv`. The script neither canonicalizes these paths nor verifies that they remain immediate children of the intended stag ...[truncated 1489 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Enforce a strict Skill-name allowlist before constructing any path: ```bash if [[ ! "$SKILL_NAME" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]] || [[ "$SKILL_NAME" == "." || "$SKILL_NAME" == ".." ]]; then echo "Invalid skill name" >&2 exit 1 fi ``` 2. Explicitly reject `/`, backslashes, control characters, and traversal components. 3. Canonicalize the parent directories with `realpath` before destructive operations. 4. Verify that each resolved target is an immediate child of the expected canonical parent: ```bash staging_root=$(realpath -m -- "$STAGING_DIR") staged_path=$(realpath -m -- "$STAGING_DIR/$SKILL_NAME") if [[ "$(dirname -- "$staged_path")" != "$staging_root" ]]; then echo "Staged path escapes staging root" >&2 exit 1 fi ``` 5. Apply equivalent containment checks to `LIVE_PATH`. 6. Reject dangerous custom roots, including empty values, `/`, the user's home directory, and the workspace root where inappropriate. 7. Before `rm -rf`, require that the target is nonempty, canonicalized, contained beneath the expected root, and not equal to the root itself. ]]>