T09 · Insecure Skill Coding Practices
Warning
- Location
- smart-restart.sh:14
- Finding
- Predictable Symlink-Unsafe Lock File Allows Local File Corruption## Vulnerability Details **File Location**: `smart-restart.sh`, lines 14 and 42–52 **Vulnerability Type**: Unsafe temporary lock file handling **Risk Level**: Medium ### Vulnerable Code ```bash LOCK_FILE="/tmp/openclaw-restart.lock" ``` ```bash check_concurrent() { if [ -f "$LOCK_FILE" ]; then LOCK_PID=$(cat "$LOCK_FILE" 2>/dev/null) if ps -p "$LOCK_PID" > /dev/null 2>&1; then error "另一个重启进程正在运行 (PID: $LOCK_PID)" error "如果确定没有其他进程，请删除锁文件: $LOCK_FILE" exit 1 else warning "发现陈旧的锁文件，正在清理..." rm -f "$LOCK_FILE" fi fi echo $$ > "$LOCK_FILE" } ``` ### Technical Analysis The script uses the fixed, predictable path `/tmp/openclaw-restart.lock` in a shared temporary directory. It creates or truncates this file using shell redirection without securely opening it, atomically claiming it, checking ownership, or rejecting symbolic links. An unprivileged local attacker can place a symbolic link at this predictable path. The test `[ -f "$LOCK_FILE" ]` follows symbolic links. If the target does not appear to contain the PID of a running process, the script removes the link; however, there is a race between that check/removal and the subsequent redirection. The attacker can recreate or replace the path with a symbolic link before: ```bash echo $$ > "$LOCK_FILE" ``` Shell redirection follows the symbolic link and truncates the linked target before writing the process ID. This is a time-of-check-to-time-of-use race combined with unsafe temporary-file handling. The lock is also not acquired atomically. Two concurrent processes can both observe that no valid lock exists and then overwrite the same file, undermining the concurrency protection that the lock is intended to provide. ### Attack Path 1. A local attacker identifies that the victim uses this Skill and monitors `/tmp/openclaw-restart.lock`. 2. T ...[truncated 1469 chars]
- Remediation
- ## Remediation Suggestions 1. Replace the predictable PID-file protocol with an advisory lock opened on a file descriptor: ```bash RUNTIME_DIR="${XDG_RUNTIME_DIR:-$HOME/.openclaw/run}" mkdir -p -- "$RUNTIME_DIR" chmod 700 -- "$RUNTIME_DIR" LOCK_FILE="$RUNTIME_DIR/openclaw-restart.lock" exec 9>"$LOCK_FILE" chmod 600 -- "$LOCK_FILE" if ! flock -n 9; then error "Another restart process is already running" exit 1 fi printf '%s\n' "$$" >&9 ``` Keep descriptor 9 open for the entire operation so the lock is released automatically when the process exits. 2. Prefer `${XDG_RUNTIME_DIR}` when available because it is normally private to the current user. Otherwise, use a dedicated directory under the user's home directory with mode `0700`, rather than a shared `/tmp` path. 3. If a filesystem lock must be created manually, use an atomic operation such as `mkdir` for lock acquisition. Verify that the lock directory is owned by the current user and is not a symbolic link. 4. Do not rely on separate existence checks followed by writes. Such checks introduce time-of-check-to-time-of-use races. 5. Apply restrictive permissions by setting an appropriate `umask`, such as `umask 077`, before creating state or lock files. 6. Add automated tests covering symlink replacement, simultaneous invocations, stale lock recovery, malformed PID contents, and interruption by signals.