T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/overseer.sh:39
- Finding
- Predictable Runtime Directory and Symlink-Unsafe State Files<![CDATA[ ## Vulnerability Details **File Location**: `scripts/overseer.sh`, lines 39–49, 107–113, and 154–161 **Vulnerability Type**: Unsafe temporary directory and state-file handling **Risk Level**: High ### Vulnerable Code ```bash default_status_dir() { if [[ -n "${XDG_RUNTIME_DIR:-}" && -d "${XDG_RUNTIME_DIR}" ]]; then echo "${XDG_RUNTIME_DIR}/overseer" else echo "/tmp/overseer-$(id -u)" fi } STATUS_DIR="${OVERSEER_DIR:-$(default_status_dir)}" ``` ```bash LOGFILE="$STATUS_DIR/overseer.log" STATUS_FILE="$STATUS_DIR/status.json" LOCKFILE="$STATUS_DIR/overseer.lock" PIDFILE="$STATUS_DIR/overseer.pid" MARKER="$STATUS_DIR/fs-marker" mkdir -p "$STATUS_DIR" chmod 700 "$STATUS_DIR" 2>/dev/null || true ``` ```bash exec 200>"$LOCKFILE" if ! flock -n 200; then echo "Another overseer is already running. Exiting." >&2 exit 0 fi echo $$ > "$PIDFILE" ``` ### Technical Analysis When `XDG_RUNTIME_DIR` is unavailable, the script falls back to the predictable path `/tmp/overseer-<UID>`. It calls `mkdir -p` without checking whether the path already existed, whether it is a real directory rather than a symlink, or whether it is owned by the current user. The subsequent `chmod` failure is explicitly ignored: ```bash chmod 700 "$STATUS_DIR" 2>/dev/null || true ``` State files are then opened by predictable names using operations that follow symbolic links. For example, `exec 200>"$LOCKFILE"` and `echo $$ > "$PIDFILE"` can truncate or overwrite the targets of symlinks. On a system where an attacker can pre-create the fallback directory with suitable permissions, and where operating-system symlink protections do not block the operation, this creates a local symlink attack. The same risk applies when an unsafe attacker-influenced directory is supplied through `--status-dir` or `OVERSEER_DIR`. ### Attack Path 1. Determine the victim user's UID and predict `/tmp/overseer-<UID>`. 2. Before the victim starts Overseer, create that direc ...[truncated 1272 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Prefer an existing, ownership-validated `XDG_RUNTIME_DIR`. 2. If a fallback is necessary, create it atomically with a non-predictable name and restrictive permissions: ```bash umask 077 STATUS_DIR="$(mktemp -d "/tmp/overseer-$(id -u).XXXXXXXX")" || exit 1 ``` 3. If a stable path is required, use `lstat` or `stat` to verify that the path: - Is a real directory and not a symlink. - Is owned by the effective UID. - Is not writable by group or other users. 4. Fail closed if directory creation, ownership validation, or permission changes fail. 5. Validate custom `--status-dir` and `OVERSEER_DIR` paths using the same rules. 6. Create state files without following symbolic links, using a helper that supports `O_NOFOLLOW` and exclusive creation where appropriate. 7. Set `umask 077` before creating any status, PID, lock, marker, log, or temporary files. 8. Add tests covering pre-existing directories, incorrect ownership, permissive modes, and symlinked state filenames. ]]>