T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/clip_save.sh:50
- Finding
- Path Traversal Allows Writes Outside the Obsidian Vault<![CDATA[ ## Vulnerability Details **File Location**: `scripts/clip_save.sh`, lines 50–203 **Vulnerability Type**: Path traversal and unrestricted filesystem write **Risk Level**: High ### Vulnerable Code ```bash month=${date:0:7} clip_dir="$VAULT/Clip/$month" mkdir -p "$clip_dir" # Basic sanitization for filenames safe_title="$title" if [[ -z "$safe_title" ]]; then safe_title="Clip" fi safe_title=$(echo "$safe_title" | tr '/:\\' '____' | tr -s ' ' ' ') # Keyword suffix for filename kw_suffix="" if [[ -n "$keywords" ]]; then kw_suffix="_${keywords}" fi file="$clip_dir/${date}_${safe_title}${kw_suffix}.md" ``` The resulting path is later used directly for file replacement, appending, or creation: ```bash if [[ -f "$file" ]]; then tmp=$(mktemp) awk -v m="$now_modified" ' BEGIN{in_fm=0} NR==1 && $0=="---" {in_fm=1; print; next} in_fm==1 && $0 ~ /^modified:/ {print "modified: " m; next} in_fm==1 && $0=="---" {in_fm=0; print; next} {print} ' "$file" > "$tmp" mv "$tmp" "$file" printf "%b" "$section" >> "$file" else { printf '%s\n' '---' printf "created: %s\n" "$date" printf "modified: %s\n" "$now_modified" printf "tags:\n%b" "$_tags_yaml" printf "category: clip\n" printf '%s\n\n' '---' printf "# %s%s\n\n" "$safe_title" "$kw_suffix" printf "%s%s\n\n" "$L_CREATED_LINE_PREFIX" "$date" printf "%b" "$section" printf "%s\n" "$_end_tags" } > "$file" fi ``` ### Technical Analysis The script partially sanitizes `title`, but it does not validate or sanitize the attacker-influenced `date` and `keywords` arguments before using them as path components. In particular: - The first seven characters of `date` are used to construct `clip_dir`. - The complete `date` value is included in the final filename. - `keywords` is appended to the filename without removing path separators or traversal sequences. - `mkdir -p`, output redirection, `mv`, and append redirection operate on the resulting unresolv ...[truncated 2199 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Strictly validate the date argument** Require the exact `YYYY-MM-DD` format and reject all other input: ```bash if [[ ! "$date" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}$ ]]; then echo "Invalid --date; expected YYYY-MM-DD" >&2 exit 2 fi ``` Also verify that the value represents a real calendar date rather than relying only on a regular expression. 2. **Apply an allowlist to keywords** Restrict keywords to the documented token format, for example: ```bash if [[ -n "$keywords" && ! "$keywords" =~ ^[A-Za-z0-9_-]+$ ]]; then echo "Invalid --keywords" >&2 exit 2 fi ``` 3. **Sanitize every filename component** Reject path separators, `..`, control characters, carriage returns, and newlines in all values used to build paths. Do not rely only on title sanitization. 4. **Enforce destination containment** Canonicalize the vault's `Clip` directory and the candidate destination, then verify that the destination starts with the canonical Clip-directory path followed by a path separator. Abort if the candidate escapes that directory. 5. **Avoid deriving directories from unchecked substrings** Construct `month` only after validating `date`: ```bash month="${date:0:7}" ``` 6. **Use atomic destination-local updates** Create temporary files securely inside the validated destination directory and atomically rename them after all containment checks. This also reduces cross-filesystem failures and race-condition exposure. 7. **Fail closed** If canonicalization, validation, directory creation, or containment verification fails, terminate without creating or modifying any file. ]]>