T09 · Insecure Skill Coding Practices
- Location
categorize.sh:23- Finding
User-Controlled Destination Path Traversal in Memory Categorization
- Content
View full analysis
Vulnerability Details
File Location:
categorize.sh, lines 23–25 and 33–72
Vulnerability Type: Path traversal leading to out-of-scope file relocation or modification
Risk Level: HighVulnerable Code
bash TYPE="$1" NAME="$2" SOURCE="$3" # Validate source file exists if [ ! -f "$SOURCE" ]; then echo "❌ Source file not found: $SOURCE" exit 1 fi # Determine destination case "$TYPE" in episodic) DEST="$MEMORY_DIR/episodic/${NAME}.md" ;; semantic) DEST="$MEMORY_DIR/semantic/${NAME}.md" ;; procedural) DEST="$MEMORY_DIR/procedural/${NAME}.md" ;; *) echo "❌ Unknown type: $TYPE" echo "Valid types: episodic, semantic, procedural" exit 1 ;; esac # Check if destination exists if [ -f "$DEST" ]; then echo "⚠️ File already exists: $DEST" echo "" read -p "Merge with existing file? (y/n) " -n 1 -r echo "" if [[ $REPLY =~ ^[Yy]$ ]]; then # Append to existing echo "" >> "$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" fiTechnical Analysis
The second command-line argument,
NAME, is interpolated directly intoDESTwithout validation or canonicalization. The script does not reject path separators,..components, absolute-path constructs, or symbolic-link-based escapes. Consequently, a value such as../../targetcan resolve outside the selected episodic, semantic, or procedural directory.The
SOURCEargument is also only checked with-f; it is not required to reside inside the expected memory or legacy directory. This allows any file readable by the invoking account to be selected.If the computed destination does not exist, `mv "$SOURCE" "$D ...[truncated 1813 chars]
- Remediation
View remediation
Remediation Suggestions
- Restrict
NAMEto a filename-safe allowlist and reject path syntax:
bash if [[ ! "$NAME" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]] || [[ "$NAME" == *".."* ]]; then echo "Invalid memory name" >&2 exit 1 fi-
Canonicalize the source and destination with
realpathand verify that they remain under explicitly approved roots. Require the source to be under"$MEMORY_DIR/legacy"or another documented import directory. -
Create and canonicalize the selected category directory before constructing the destination. Reject any destination whose canonical parent differs from that directory.
-
Refuse symbolic links for both source and destination, or resolve them and repeat the containment checks after resolution.
-
Use option terminators for filesystem commands:
bash mv -- "$SOURCE_REAL" "$DEST" cat -- "$SOURCE_REAL" >> "$DEST"-
Use a temporary file followed by an atomic rename for merge operations. This reduces partial-file corruption if an operation fails.
-
Avoid interactive confirmation when invoked by unattended agent automation. Require an explicit merge flag and perform all containment validation regardless of that flag.
-
Run the skill with least-privilege filesystem access so it can read and write only the intended memory hierarchy.
- Restrict