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-8, 29, 32-40, 52-76` **Vulnerability Type**: Plaintext storage of potentially sensitive user input **Risk Level**: Medium ### Vulnerable Code ```bash DATA_DIR="${GRAMMAR_CHECK_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/grammar-check}" DB="$DATA_DIR/data.log" mkdir -p "$DATA_DIR" ``` ```bash _log() { echo "$(date '+%m-%d %H:%M') $1: $2" >> "$DATA_DIR/history.log"; } cmd_run() { echo " Running: $1" _log "run" "${1:-}" } cmd_config() { echo " Config: $DATA_DIR/config.json" _log "config" "${1:-}" } cmd_status() { echo " Status: ready" _log "status" "${1:-}" } ``` ```bash cmd_add() { echo "$(date +%Y-%m-%d) $*" >> "$DB"; echo " Added: $*" _log "add" "${1:-}" } cmd_remove() { echo " Removed: $1" _log "remove" "${1:-}" } cmd_search() { grep -i "$1" "$DB" 2>/dev/null || echo " Not found: $1" _log "search" "${1:-}" } cmd_export() { [ -f "$DB" ] && cat "$DB" || echo "No data" _log "export" "${1:-}" } cmd_info() { echo " Version: $VERSION | Data: $DATA_DIR" _log "info" "${1:-}" } ``` ### Technical Analysis The script persistently records raw command arguments in `data.log` and `history.log`. Arguments supplied to commands such as `run`, `add`, and `search` may contain private text, search terms, or other sensitive information. The declared grammar-checking documentation does not disclose this persistence or define retention and deletion behavior. The script creates the data directory and files without explicitly applying restrictive permissions. Their effective permissions therefore depend on the invoking user's `umask`. With a commonly used `022` umask, the directory can be traversable and the files can be readable by other local users. The `_log` function also writes raw arguments with `echo`. An argument containing newline characters can create forged or misleading log entries because no escaping or structured ...[truncated 1253 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove persistent logging unless it is necessary for the documented grammar-checking functionality. 2. Obtain explicit user consent before retaining command arguments and clearly document the purpose, storage location, retention period, export behavior, and deletion procedure. 3. Never log raw user text by default. Record only non-sensitive operational metadata or redact argument values. 4. Create the storage directory and files with restrictive permissions: ```bash umask 077 mkdir -p -m 700 -- "$DATA_DIR" touch "$DB" "$DATA_DIR/history.log" chmod 600 -- "$DB" "$DATA_DIR/history.log" ``` 5. Normalize or escape carriage returns and newline characters before writing any user-controlled value to a line-oriented log. 6. Prefer a structured logging format with safe serialization rather than constructing records with `echo`. 7. Add a command that securely clears retained data, and consider automatic expiration for historical records. 8. Align `scripts/script.sh` with the behavior documented in `SKILL.md`, or remove the unrelated generic datastore functionality from the package. ]]>