T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/create-task.sh:20
- Finding
- Unvalidated task identifiers allow path traversal outside the task directories<![CDATA[ ## Vulnerability Details **File Location**: `scripts/create-task.sh:20-45`; `scripts/archive-task.sh:16-54` **Vulnerability Type**: Path traversal and unrestricted filesystem access **Risk Level**: High ### Vulnerable Code From `scripts/create-task.sh`: ```bash TASK_ID="$1" TITLE="$2" PRIORITY="$3" DOMAIN="$4" DUE_DATE="${5:-—}" DESCRIPTION="${6:-}" TODAY=$(date +%Y-%m-%d) if [ -z "$TASK_ID" ] || [ -z "$TITLE" ] || [ -z "$PRIORITY" ] || [ -z "$DOMAIN" ]; then echo "Usage: create-task.sh <task_id> <title> <priority> <domain> [due_date] [description]" >&2 exit 1 fi # Ensure active directory exists mkdir -p "$ACTIVE_DIR" FILE="$ACTIVE_DIR/${TASK_ID}.md" if [ -f "$FILE" ]; then echo "Error: $FILE already exists" >&2 exit 1 fi cat > "$FILE" << EOF # ${TASK_ID}: ${TITLE} ``` From `scripts/archive-task.sh`: ```bash TASK_ID="$1" NEW_STATUS="$2" REASON="${3:-状态变更: $NEW_STATUS → 移入 archived}" TODAY=$(date +%Y-%m-%d) if [ -z "$TASK_ID" ] || [ -z "$NEW_STATUS" ]; then echo "Usage: archive-task.sh <task_id> <new_status> [reason]" >&2 echo " new_status: Done | Cancelled" >&2 exit 1 fi if [[ "$NEW_STATUS" != "Done" && "$NEW_STATUS" != "Cancelled" ]]; then echo "Error: new_status must be Done or Cancelled" >&2 exit 1 fi SRC="$ACTIVE_DIR/${TASK_ID}.md" if [ ! -f "$SRC" ]; then echo "Error: $SRC not found" >&2 exit 1 fi # Ensure archive directory exists mkdir -p "$ARCHIVE_DIR" # Update status in Metadata sed -i "s/^| 状态 | .* |$/| 状态 | ${NEW_STATUS} |/" "$SRC" # Update 最后更新 sed -i "s/^| 最后更新 | .* |$/| 最后更新 | ${TODAY} |/" "$SRC" # Append changelog entry (before the last line or at end) echo "| ${TODAY} | ${REASON} |" >> "$SRC" # Move to archived DST="$ARCHIVE_DIR/${TASK_ID}.md" mv "$SRC" "$DST" ``` ### Technical Analysis Both scripts document the task identifier as having the form `TASK-XXXXX`, but they only check whether the argument is nonempty. The value is directly interpolated into filesystem paths. A task identifier cont ...[truncated 2317 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Enforce the documented task identifier format before constructing any path: ```bash if [[ ! "$TASK_ID" =~ ^TASK-[0-9]{5}$ ]]; then echo "Error: task_id must match TASK-XXXXX" >&2 exit 1 fi ``` 2. Canonicalize both the base directory and candidate path, then verify that the candidate remains below the expected directory: ```bash base=$(realpath -m -- "$ACTIVE_DIR") candidate=$(realpath -m -- "$ACTIVE_DIR/${TASK_ID}.md") case "$candidate" in "$base"/*) ;; *) echo "Error: resolved path escapes active directory" >&2 exit 1 ;; esac ``` 3. Apply an equivalent boundary check to both source and destination paths in `archive-task.sh`. 4. Reject symbolic-link destinations and sources where appropriate. For new files, use exclusive creation rather than a separate `-f` check followed by redirection. 5. Add `set -euo pipefail` so failed edits or moves stop execution immediately. 6. Make archive operations transactional where possible: write the updated content to a securely created temporary file inside the destination filesystem, verify it, and atomically rename it. 7. Add regression tests covering identifiers with `../`, absolute paths, path separators, malformed numeric portions, oversized values, and symbolic links. ]]>