T09 · Insecure Skill Coding Practices
- Location
scripts/export-agent-core-pack.sh:163- Finding
Unvalidated Path Components Permit Writes Outside the Intended Staging Directory
- Content
View full analysis
&2; usage >&2; exit 1 ;; esac done require_cmd zip BASE_DIR="$(resolve_path "$BASE_DIR")" OUTPUT_DIR="$(resolve_path "$OUTPUT_DIR")" STAGING_DIR="$OUTPUT_DIR/$NAME" ZIP_PATH="$OUTPUT_DIR/$NAME.zip" [[ ! -e "$STAGING_DIR" ]] || { echo "Refusing to overwrite: $STAGING_DIR" >&2; exit 1; } [[ ! -e "$ZIP_PATH" ]] || { echo "Refusing to overwrite: $ZIP_PATH" >&2; exit 1; } mkdir -p "$STAGING_DIR" declare -a INCLUDED_PATHS=() declare -a FRAMEWORK_LINES=() IFS=',' read -r -a REPOS <<<"$REPOS_RAW" for repo in "${REPOS[@]}"; do repo="$(printf '%s' "$repo" | xargs)" [[ -n "$repo" ]] || continue repo_dir="$BASE_DIR/$repo" [[ -d "$repo_dir" ]] || { echo "Missing repo: $repo_dir" >&2; exit 1; } framework="$(detect_framework "$repo_dir")" [[ "$framework" != "unknown" ]] || { echo "Unsupported framework in $repo" >&2; exit 1; } FRAMEWORK_LINES+=("$repo | $framework") mkdir -p "$STAGING_DIR/$repo" extract_by_framework "$repo" "$repo_dir" "$framework" "$STAGING_DIR/$repo" done ``` The resulting destination is subsequently passed to overwrite-capable copy operations: ```bash copy_file() { local src="$1" local dst="$2" [[ -f "$src" ]] || return 0 mkdir -p "$(dirname "$dst")" cp -p "$src" "$dst" INCLUDED_PATHS+=("$src") } copy_dir() { local src="$1" local dst="$2" [[ -d "$src" ]] || return 0 mkdir -p "$(dirname "$dst")" cp -Rp "$src" "$dst" INCLUDED_PATHS+=("$src") } ``` ### Technical Analysis The user-controlled `NAME` value and eac ...[truncated 2305 chars]- Remediation
View remediation
&2 exit 1 fi } ``` 2. Validate `NAME` after option parsing and validate every repository identifier before using it: ```bash validate_component "$NAME" "archive name" for repo in "${REPOS[@]}"; do repo="$(printf '%s' "$repo" | xargs)" [[ -n "$repo" ]] || continue validate_component "$repo" "repository name" # Continue processing. done ``` 3. Canonicalize output and destination paths and verify containment before creating directories or copying files. The canonical staging path must remain beneath the canonical output directory, and every extraction destination must remain beneath the canonical staging directory. 4. Explicitly reject absolute paths, `/`, backslashes where applicable, and all `..` path components even if a broader naming policy is later adopted. 5. Use non-overwriting copy behavior, such as `cp -n`, or explicitly fail when a destination file already exists. This provides defense in depth but should not replace containment validation. 6. Consider creating the staging directory with `mktemp -d` beneath the validated output directory to reduce path collisions and unsafe directory reuse. ]]>
