T09 · Insecure Skill Coding Practices
- Location
categorize.sh:23- Finding
Path Traversal in Memory Categorization Destination
- Content
View full analysis
> "$DEST" echo "---" >> "$DEST" echo "# Merged from: $(basename "$SOURCE")" >> "$DEST" echo "# Date: $(date +"%Y-%m-%d %H:%M:%S")" >> "$DEST" echo "" >> "$DEST" cat "$SOURCE" >> "$DEST" echo "✅ Merged into: $DEST" else echo "❌ Cancelled" exit 1 fi else # Move to destination mv "$SOURCE" "$DEST" echo "✅ Categorized as $TYPE: $DEST" fi ``` ### Technical Analysis The user-controlled `NAME` argument is interpolated directly into `DEST`. The script does not reject directory separators, `..` path components, absolute paths, or other path-like input. Although the destination is prefixed with a category directory and suffixed with `.md`, traversal components can escape the intended directory. For example: ```text NAME=../../target DEST=$MEMORY_DIR/semantic/../../target.md ``` After path resolution, this destination is outside `memory/semantic`. Additional traversal components can escape further up the filesystem hierarchy. The `SOURCE` argument is also unrestricted apart from the requirement t ...[truncated 1837 chars]- Remediation
View remediation
&2 exit 1 fi ``` 2. Explicitly reject path separators: ```bash case "$NAME" in *"/"*|*"\\"*) echo "Path separators are not permitted" >&2; exit 1 ;; esac ``` 3. Canonicalize and validate the destination after selecting the category: ```bash category_dir="$MEMORY_DIR/$TYPE" mkdir -p -- "$category_dir" category_real=$(realpath -- "$category_dir") dest_real=$(realpath -m -- "$category_dir/${NAME}.md") case "$dest_real" in "$category_real"/*) ;; *) echo "Destination escapes category directory" >&2; exit 1 ;; esac ``` 4. If the source is intended to be a memory file, canonicalize it and require it to remain beneath an approved directory such as `memory/legacy`. 5. Use `mv -- "$SOURCE" "$DEST"` and `cat -- "$SOURCE"` to prevent option interpretation and make the safety intent explicit. 6. Avoid interactive merging in automated agent workflows. Require an explicit `--merge` option and validate the destination before any append operation. ]]>
