T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/script.sh:6
- Finding
- Undisclosed Plaintext Persistence of User-Supplied Arguments## Vulnerability Details **File Location**: `scripts/script.sh:6-9, 34, 36-38, 55-58` **Vulnerability Type**: Undisclosed plaintext storage of potentially sensitive user input **Risk Level**: Medium ### Vulnerable Code ```bash DATA_DIR="${COVER_LETTER_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/cover-letter}" 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:-}" } ``` ```bash cmd_add() { echo "$(date +%Y-%m-%d) $*" >> "$DB"; echo " Added: $*" _log "add" "${1:-}" } ``` ### Technical Analysis The launcher creates persistent storage under the user's data directory on every invocation. It then writes command arguments to `history.log`; the `add` command additionally writes the complete argument list to `data.log`. The project documentation presents the tool as a cover-letter generator and directs users to invoke `cover-letter run`. Users may consequently supply names, employers, job titles, contact information, employment history, or other personal data. The script does not disclose that arguments are retained, request consent, define a retention policy, redact sensitive fields, or establish restrictive file permissions. The files are created using the process's existing `umask`. On systems with permissive defaults, other local accounts may be able to read the stored information. The append operations also do not verify that `history.log` and `data.log` are regular files rather than symbolic links. If an attacker can modify the data directory—for example, through an unsafe custom `COVER_LETTER_DIR`, prior directory compromise, or another process running as the same user—the attacker can redirect appended content to another file writable by the victim. This behavior is especially concerning because `scripts/script.sh` does not implement the advertised cover-letter generation functionality; it behaves as a generi ...[truncated 1571 chars]
- Remediation
- ## Remediation Suggestions 1. Remove the generic logging and CRUD subsystem from the cover-letter launcher unless persistent storage is necessary for its documented purpose. 2. Do not log raw command arguments. Record only non-sensitive operational metadata, or redact names, contact details, employers, and free-form content. 3. Clearly disclose all persistent storage behavior and obtain explicit user consent before retaining cover-letter data. 4. Apply restrictive permissions before creating storage: ```bash umask 077 mkdir -p -- "$DATA_DIR" chmod 700 -- "$DATA_DIR" ``` 5. Create data files with mode `0600` and verify ownership before every write. 6. Reject symbolic links and non-regular files. Prefer securely opened file descriptors with no-follow protections where supported rather than shell redirection to attacker-influenced paths. 7. Validate `COVER_LETTER_DIR` and avoid using a directory that is shared, group-writable, world-writable, or owned by another user. 8. Implement an explicit deletion command and a documented retention period. 9. Align `scripts/script.sh` with the documented cover-letter functionality, or update the documentation so users are not misled about what `cover-letter run` executes.