T09 · Insecure Skill Coding Practices
- Location
categorize.sh:23- Finding
Unrestricted source and destination paths allow filesystem operations outside the memory workspace
- 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 script accepts both `NAME` and `SOURCE` directly from command-line arguments. Although shell quoting prevents ordinary shell command injection, the values are not restricted to the intended memory hierarchy. `NAME` is concatenated into a destination path without rejecting path separators or `..` components. A value containing traversal sequences can therefore cause the normalized destination to resolve outside `$MEMORY_DIR/episodic`, `$MEMORY_DIR/semantic`, or `$MEMORY_DIR/procedural`. `SOURCE` is only checked with `-f`. It may consequently identify any regular file accessible to the invoking user, rather than a file within the memor ...[truncated 2189 chars]- Remediation
View remediation
&2 exit 1 fi ``` 2. Reject names containing `/`, backslashes, traversal components, control characters, or leading option-like values. 3. Canonicalize and validate the source path before using it. Require it to remain beneath an approved directory such as `$MEMORY_DIR/legacy`: ```bash source_real=$(realpath -- "$SOURCE") || exit 1 allowed_real=$(realpath -- "$MEMORY_DIR/legacy") || exit 1 case "$source_real" in "$allowed_real"/*) ;; *) echo "Source must be inside $allowed_real" >&2 exit 1 ;; esac ``` 4. Canonicalize the destination parent and verify that it exactly matches the selected category directory before writing. 5. Reject symbolic-link sources and destinations where symlink behavior is not explicitly required. 6. Use `mv -- "$source_real" "$DEST"` and similar `--` separators for filesystem commands. 7. Avoid appending directly to an existing file. Create a temporary file securely in the destination directory, validate the result, and atomically rename it into place. 8. Add automated tests covering `../`, absolute paths, symbolic links, whitespace, control characters, and sources outside the memory directory. ]]>
