T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/script.sh:5
- Finding
- Undisclosed Persistent Plaintext Storage of User-Supplied Data## Vulnerability Details **File Location**: `scripts/script.sh:5-8`, `scripts/script.sh:30`, `scripts/script.sh:52-53`, and `scripts/script.sh:88-89` **Vulnerability Type**: Persistent plaintext storage and logging of potentially sensitive input **Risk Level**: Medium ### Vulnerable Code ```bash DATA_DIR="${REVIEW_RESPONDER_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/review-responder}" 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 run) shift; cmd_run "$@" ;; add) shift; cmd_add "$@" ;; ``` ### Technical Analysis The script creates a persistent data directory whenever it runs. Its `add` command stores all supplied arguments in `data.log`, while `_log` records command data in `history.log`. The `run` command also passes its first user-supplied argument to `_log`. The Skill is presented as a customer-review response assistant, so arguments may reasonably contain review text, customer identifiers, order details, complaint information, or other business data. The documentation does not disclose that the generic utility persists such input. The script does not redact sensitive fields, request explicit storage consent, define retention limits, or establish restrictive permissions through `umask 077` or explicit `chmod` operations. Resulting access permissions therefore depend on the caller's environment and inherited umask. Although `cmd_remove` reports that an item was removed, it does not alter either stored file: ```bash cmd_remove() { echo " Removed: $1" _log "remove" "${1:-}" } ``` Consequently, users may incorrectly believe that stored information has been deleted. ### Attack Path 1. A user or agent invokes `scripts/script.sh` and supplies customer-review or order-rela ...[truncated 1484 chars]
- Remediation
- ## Remediation Suggestions 1. Remove `scripts/script.sh` if it is unrelated to the declared review-response functionality. 2. Require explicit user consent before storing any supplied content, and clearly document the storage path, purpose, format, and retention period. 3. Avoid recording raw command arguments. Log only non-sensitive event metadata or redact customer identifiers, review content, order information, credentials, and tokens. 4. Create storage with restrictive permissions before writing files: ```bash umask 077 mkdir -p -- "$DATA_DIR" touch -- "$DB" "$DATA_DIR/history.log" chmod 700 -- "$DATA_DIR" chmod 600 -- "$DB" "$DATA_DIR/history.log" ``` 5. Implement a functional deletion command that identifies and removes the requested record, verifies the result, and returns a failure status if deletion does not occur. 6. Add retention controls that automatically expire historical records and provide an explicit command to erase all stored data. 7. Validate `REVIEW_RESPONDER_DIR` before use and document that changing it can direct stored data to another filesystem location. 8. Add tests confirming that sensitive arguments are not logged, files receive restrictive permissions, and deletion actually removes the selected data.