T09 · Insecure Skill Coding Practices
- Location
astrill-watchdog.sh:223- Finding
Unsafe PID File Trust and Broad Process Matching Can Terminate Unrelated Processes
- Content
View full analysis
Vulnerability Details
File Location:
astrill-watchdog.sh:223-256
Vulnerability Type: Unsafe process identification and termination
Risk Level: MediumThe standalone lifecycle commands trust a PID read from a persistent file without verifying the identity of the referenced process. They also use broad full-command-line matching to locate and terminate presumed orphan watchdog processes.
Vulnerable code in
cmd_start:bash cmd_start() { if [[ -f "$PID_FILE" ]]; then local old old="$(cat "$PID_FILE" 2>/dev/null)" || old="" if [[ -n "$old" ]] && kill -0 "$old" 2>/dev/null; then echo "Watchdog already running (PID ${old})." exit 0 fi rm -f "$PID_FILE" fi local orphans orphans="$(pgrep -f 'astrill-watchdog.sh _loop' 2>/dev/null || true)" if [[ -n "$orphans" ]]; then echo "Cleaning up orphaned loop processes: ${orphans}" echo "$orphans" | xargs kill 2>/dev/null || true sleep 1 fi nohup bash "$0" _loop >> "$LOG_FILE" 2>&1 & local new_pid="$!" echo "$new_pid" > "$PID_FILE" chmod 600 "$PID_FILE" echo "Watchdog started (PID ${new_pid}). Log: ${LOG_FILE}" }Vulnerable code in
cmd_stop:bash cmd_stop() { local killed=0 if [[ -f "$PID_FILE" ]]; then local pid pid="$(cat "$PID_FILE" 2>/dev/null)" || pid="" if [[ -n "$pid" ]] && kill "$pid" 2>/dev/null; then echo "Watchdog (PID ${pid}) stopped." killed=1 fi rm -f "$PID_FILE" fi local orphans orphans="$(pgrep -f 'astrill-watchdog.sh _loop' 2>/dev/null || true)" if [[ -n "$orphans" ]]; then echo "$orphans" | xargs kill 2>/dev/null || true echo "Killed orphaned loop processes: ${orphans}" killed=1 ...[truncated 2638 chars]- Remediation
View remediation
Remediation Suggestions
-
Use systemd as the sole lifecycle authority when the watchdog is installed as a user service. Replace manual PID discovery and termination with:
bash systemctl --user start astrill-watchdog.service systemctl --user stop astrill-watchdog.service systemctl --user is-active astrill-watchdog.service -
If standalone operation must remain, validate PID-file contents before using them:
- Require a strictly numeric value.
- Reject PID values less than or equal to 1.
- Confirm
/proc/$pidexists. - Verify
/proc/$pid/exeresolves to the expected Bash executable or installed watchdog. - Verify the NUL-separated
/proc/$pid/cmdlinecontains the exact installed script path and_loopargument. - Compare the process start time against a start-time value stored alongside the PID to detect PID reuse.
-
Use an exclusive lock, such as
flock, held for the lifetime of the loop rather than relying on PID-file existence:bash exec 9>"$LOG_DIR/watchdog.lock" flock -n 9 || exit 0 -
Remove
pgrep -f | xargs kill. If orphan recovery is required, enumerate candidate PIDs and independently validate each process's executable, exact argument vector, user ID, and installed script path before sending a signal. -
After validation, signal one PID at a time using
kill -- "$pid"and recheck its identity immediately before signaling to reduce time-of-check/time-of-use exposure. -
Remove the PID file with an
EXIT,TERM, andINTtrap when running in standalone mode, while retaining identity validation because abrupt termination can bypass cleanup.
-