T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/lib.sh:28- Finding
Unvalidated Version Argument Allows Cache Directory Path Traversal
- Content
View full analysis
/dev/null | wc -l | tr -d ' ') rm -f "${VERSION_CACHE_DIR}"/doc_*.txt \ "${VERSION_CACHE_DIR}"/doc_*.md \ "${VERSION_CACHE_DIR}/index.txt" \ "${VERSION_CACHE_DIR}/index_meta.json" echo "Cleared $count cached docs and index from version: $VERSION" ;; ``` ### Technical Analysis The `--version` argument is accepted as an arbitrary string and appended directly to `CACHE_DIR`. The implementation does not reject path separators, `..` components, absolute-path syntax, or other malformed version identifiers. Quoting the resulting variable prevents shell word splitting, but it does not prevent filesystem path traversal. For example, a version value such as `../../target` produces a path equivalent to: ```text /../../target ``` The scripts subsequently use that escaped path for directory creation, cache reads and writes, index generation, and deletion. In particular, `cache.sh clear-docs` deletes fixed filenames and matching `doc_*.txt` ...[truncated 1706 chars]- Remediation
View remediation
&2 return 1 fi } ``` 2. Explicitly reject `/`, `\`, empty values, `.` components, and `..` components regardless of platform. 3. Canonicalize both the cache root and candidate directory, then verify that the candidate remains beneath the cache root before creating, reading, writing, or deleting files: ```bash cache_root="$(cd "$CACHE_DIR" && pwd -P)" candidate="${cache_root}/${VERSION}" mkdir -p "$candidate" candidate="$(cd "$candidate" && pwd -P)" case "$candidate" in "$cache_root"/*) ;; *) echo "Error: version cache path escapes cache root" >&2 exit 1 ;; esac VERSION_CACHE_DIR="$candidate" ``` 4. Apply the containment check again immediately before destructive operations such as `clear-docs`, rather than relying solely on validation performed earlier. 5. Consider resolving user-facing version tags to an internal safe identifier, such as a validated slug or cryptographic hash, instead of using the raw argument as a directory name. 6. Add regression tests covering `../`, nested traversal, absolute paths, backslashes, `.`, `..`, malformed tags, and valid release names. Tests should verify that no directory or file outside `CACHE_DIR` is created, modified, or deleted. ]]>