T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/script.sh:6
- Finding
- Undisclosed Plaintext Persistence of User-Supplied Data<![CDATA[ ## Vulnerability Details **File Location**: `scripts/script.sh:6-9`, `scripts/script.sh:34`, `scripts/script.sh:58-60`, `scripts/script.sh:67-69`, and `scripts/script.sh:76-79` **Vulnerability Type**: Undisclosed plaintext storage, insufficient filesystem hardening, and unnecessary data retention **Risk Level**: Medium ### Vulnerable Code ```bash DATA_DIR="${PRODUCT_DESC_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/product-desc}" DB="$DATA_DIR/data.log" mkdir -p "$DATA_DIR" ``` ```bash _log() { echo "$(date '+%m-%d %H:%M') $1: $2" >> "$DATA_DIR/history.log"; } ``` ```bash cmd_add() { echo "$(date +%Y-%m-%d) $*" >> "$DB"; echo " Added: $*" _log "add" "${1:-}" } ``` ```bash cmd_search() { grep -i "$1" "$DB" 2>/dev/null || echo " Not found: $1" _log "search" "${1:-}" } ``` ```bash cmd_export() { [ -f "$DB" ] && cat "$DB" || echo "No data" _log "export" "${1:-}" } ``` ### Technical Analysis The skill is documented as a product-description, SEO, comparison, and localization helper. However, `scripts/script.sh` implements an unrelated persistent data utility that creates a local data directory and records caller-supplied values in plaintext. The `add` command writes all supplied arguments to `data.log`. The `_log` function also appends command names and selected arguments to `history.log`. Search terms are therefore retained as well. This storage behavior is not disclosed in `SKILL.md`, and the implementation provides no retention limits, sensitive-data warning, access-control initialization, or secure deletion mechanism. The directory and files inherit permissions from the executing process's current `umask`. The script does not enforce a restrictive mode such as owner-only access. On systems with permissive defaults, shared accounts, backups, or other local readers, retained product information may become accessible beyond its intended audience. The `PRODUCT_DESC_DIR` environment variable permits storage to be redirected ...[truncated 1946 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove `scripts/script.sh` if it is unrelated boilerplate and route the documented product-description commands exclusively through `scripts/desc.sh`. 2. If persistent storage is genuinely required, document exactly what is stored, why it is stored, where it is stored, and how users can inspect and delete it. 3. Enforce restrictive permissions before creating storage: ```bash umask 077 install -d -m 700 -- "$DATA_DIR" ``` 4. Create data files with owner-only permissions and verify that existing targets are regular files owned by the expected user. 5. Reject symbolic links and unexpected file types before writing. Where practical, use file-opening mechanisms that provide no-follow semantics. 6. Validate or constrain `PRODUCT_DESC_DIR`; resolve its canonical parent and reject unsafe, shared, or unexpected paths. 7. Do not log raw command arguments by default. Record only non-sensitive operational metadata, or make logging explicit and opt-in. 8. Implement retention limits and an actual deletion command that removes requested records rather than only printing a success message. 9. Clearly warn users not to submit credentials, personal data, confidential product information, or other secrets to commands that persist input. 10. Add automated tests covering restrictive permissions, symbolic-link rejection, path validation, data deletion, and disabled-by-default argument logging. ]]>