T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/generate_summary.sh:18
- Finding
- Arbitrary File Overwrite Through Unsanitized Output Path<![CDATA[ ## Vulnerability Details **File Location**: `scripts/generate_summary.sh`, lines 18-28 and 44 **Vulnerability Type**: Path traversal and arbitrary file overwrite **Risk Level**: Medium ### Complete Vulnerable Code ```bash OUTPUT_DIR="${2:-reflections}" if [ -z "$LABEL" ]; then echo "Usage: bash generate_summary.sh <session-label> [output-dir]" echo "Example: bash generate_summary.sh '2025-06-15-first-session'" exit 1 fi mkdir -p "$OUTPUT_DIR" OUTPUT_FILE="${OUTPUT_DIR}/${LABEL}.md" ``` The resulting path is later opened with truncating redirection: ```bash cat > "$OUTPUT_FILE" << EOFSYNOPSIS # 魔镜 Reflection: ${LABEL} **Date:** $(date '+%Y-%m-%d %H:%M') **Session Label:** ${LABEL} ``` ### Technical Analysis The script directly incorporates the attacker-controllable `LABEL` argument into `OUTPUT_FILE` without validating that it is a simple filename. A label can contain directory separators or traversal components such as `../`. The shell quoting prevents command injection, but it does not prevent filesystem path traversal. The `>` operator creates the selected file or truncates it if it already exists. It also follows symbolic links. Consequently, the script can overwrite any `.md` path writable by the account running it, provided the supplied traversal path resolves to that destination. The independently supplied `OUTPUT_DIR` also allows the caller to select a destination directory. Although that behavior is documented, combining it with an unrestricted label and unconditional truncation increases the risk of unintended file modification. ### Attack Path 1. An attacker gains control over, or persuades a user or automation process to use, the script's `session-label` argument. 2. The attacker supplies a traversal label, for example: ```bash bash scripts/generate_summary.sh "../../project/README" ``` 3. The script constructs a path equivalent to: ```text reflections/../../project/README.md ``` 4. `cat > "$OU ...[truncated 822 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Restrict `LABEL` to a safe basename: ```bash if [[ ! "$LABEL" =~ ^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$ ]] || [[ "$LABEL" == *".."* ]]; then echo "Invalid session label" >&2 exit 1 fi ``` 2. Explicitly reject `/`, `\`, control characters, and traversal components. 3. Canonicalize the output directory and candidate parent path, then verify that the destination remains beneath the approved output directory. 4. Refuse to overwrite existing files unless the caller supplies a deliberate overwrite option: ```bash set -o noclobber : > "$OUTPUT_FILE" || { echo "Output file already exists" >&2 exit 1 } ``` 5. Check for symbolic links before writing, and use a safe file-creation mechanism that does not follow links where supported. 6. If arbitrary output directories are unnecessary, remove the second argument and use a fixed, application-controlled directory. ]]>