T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/suggest-compact.sh:20
- Finding
- Predictable Temporary File Handling Enables File Clobbering and Arithmetic Injection## Vulnerability Details **File Location**: `scripts/suggest-compact.sh`, lines 20–31 **Vulnerability Type**: Unsafe temporary file handling and evaluation of untrusted counter data **Risk Level**: High ### Vulnerable Code ```bash SESSION_ID="${CLAUDE_SESSION_ID:-${PPID:-default}}" COUNTER_FILE="/tmp/claude-tool-count-${SESSION_ID}" THRESHOLD=${COMPACT_THRESHOLD:-50} # Initialize or increment counter if [ -f "$COUNTER_FILE" ]; then count=$(cat "$COUNTER_FILE") count=$((count + 1)) echo "$count" > "$COUNTER_FILE" else echo "1" > "$COUNTER_FILE" count=1 fi ``` ### Technical Analysis The script creates a predictable state-file path under the shared `/tmp` directory. The path incorporates `CLAUDE_SESSION_ID` without validating or normalizing it. Consequently, path separators and traversal sequences in a caller-controlled session ID can redirect the counter file to another location writable by the executing account. The script also does not securely create the counter file or verify its ownership, type, and link status. The `[ -f "$COUNTER_FILE" ]` test follows symbolic links, and the subsequent output redirection is not protected against link replacement. A local attacker who can predict the session identifier can pre-create the counter path as a symbolic link. When the script runs, `echo "$count" > "$COUNTER_FILE"` follows that link and truncates or overwrites its target. There is also no validation of the existing file contents before they are used in Bash arithmetic: ```bash count=$(cat "$COUNTER_FILE") count=$((count + 1)) ``` Bash arithmetic expressions can recursively interpret variable values as arithmetic syntax. Carefully crafted content, including expressions that trigger shell expansion through constructs such as malicious array subscripts, may cause unintended command execution when evaluated. At minimum, malformed content can reliably trigger errors and disrupt hook execution. ` ...[truncated 2183 chars]
- Remediation
- ## Remediation Suggestions 1. Validate `CLAUDE_SESSION_ID` using a strict allowlist before using it in a path. For example, permit only a bounded sequence of ASCII letters, digits, periods, underscores, and hyphens. Reject invalid or excessively long identifiers rather than attempting to sanitize them. 2. Create a private runtime directory owned by the executing user with mode `0700`. Prefer `$XDG_RUNTIME_DIR` when it is available and trustworthy; otherwise create a directory securely with `mktemp -d` and restrictive permissions. 3. Do not use a predictable file directly in the shared `/tmp` namespace. If persistent naming is required, verify that the containing directory and counter file are owned by the expected user and are not symbolic links. 4. Use atomic and race-resistant state updates. Open files with no-follow and exclusive-creation semantics where possible, lock the state during read-modify-write operations, write through a securely created temporary file, and atomically rename it into place. 5. Require the counter content to match `^[0-9]+$` before performing arithmetic. Reject or safely reset invalid, negative, or excessively large values. 6. Validate `COMPACT_THRESHOLD` as a bounded positive decimal integer before using it in numeric comparisons or arithmetic expressions. 7. Apply restrictive permissions with `umask 077` before creating state files. 8. Avoid running the script with elevated privileges. If cron or hook integration is used, execute it under the least-privileged account necessary. 9. Add tests covering symbolic links, path traversal, malformed counter values, malicious arithmetic expressions, concurrent invocations, invalid thresholds, and oversized inputs.