T09 · Insecure Skill Coding Practices
- Location
scripts/lib/nodes.sh:25- Finding
Untrusted PID File Can Terminate an Unrelated Same-User Process
- Content
View full analysis
Vulnerability Details
File Location:
scripts/lib/nodes.sh, lines 25–42
Vulnerability Type: Untrusted PID-file handling and insufficient process identity validation
Risk Level: MediumVulnerable Code
bash stop_bg() { local name="$1" local pid_file="$PID_DIR/$name.pid" if [[ ! -f "$pid_file" ]]; then echo "$name not running (no pid file)" return 0 fi local pid pid="$(cat "$pid_file" 2>/dev/null || true)" if [[ -n "$pid" ]] && kill -0 "$pid" 2>/dev/null; then kill "$pid" echo "Stopped $name (pid $pid)" else echo "$name not running (stale pid)" fi rm -f "$pid_file" }Related PID-directory configuration in
scripts/peaq_ros2.sh, lines 79–80:bash LOG_DIR="${PEAQ_ROS2_LOG_DIR:-$HOME/.peaq_ros2/logs${ROS_DOMAIN_SUFFIX}}" PID_DIR="${PEAQ_ROS2_PID_DIR:-$HOME/.peaq_ros2/pids${ROS_DOMAIN_SUFFIX}}"The same trust issue affects the existing-process check in
scripts/lib/nodes.sh, lines 7–18:bash local pid_file="$PID_DIR/$name.pid" if [[ -f "$pid_file" ]]; then local existing_pid existing_pid="$(cat "$pid_file" 2>/dev/null || true)" if [[ -n "$existing_pid" ]] && kill -0 "$existing_pid" 2>/dev/null; then if ps -o stat= -p "$existing_pid" 2>/dev/null | grep -q "Z"; then rm -f "$pid_file" else echo "$name already running (pid $existing_pid)" return 0 fi fi fiTechnical Analysis
The process-management implementation treats the contents of a predictable PID file as authoritative. Before sending
SIGTERM,stop_bgonly checks that the value is nonempty and refers to a process visible tokill -0. It does not verify:- That the file contains a strictly valid positive numeric PID.
- That the PID belongs to the ROS node represented by the filename.
- That the process start time matches the process originally launched by the Skill.
- That the PID fi ...[truncated 2461 chars]
- Remediation
View remediation
Remediation Suggestions
-
Validate PID syntax before using it:
bash [[ "$pid" =~ ^[1-9][0-9]*$ ]] || fatal "Invalid PID file contents" -
Verify process identity before signaling it. Compare
/proc/$pid/cmdline,/proc/$pid/exe, and the process start time against metadata recorded when the node was launched. Do not rely only on a command-name substring because it can be spoofed. -
Store more than a PID. Record the PID, process start time, expected executable, and expected arguments in a protected state file, then require all fields to match before stopping the process.
-
Protect the state directory and files explicitly:
bash umask 077 install -d -m 700 -- "$PID_DIR"Create PID files atomically with mode
0600, and reject directories or files not owned by the current user. -
Canonicalize and constrain
PEAQ_ROS2_PID_DIRto an approved private root. Reject symbolic links and unsafe parent-directory ownership or permissions. -
Validate node-name overrides before incorporating them into filenames. Use a narrow allowlist such as alphanumeric characters, underscores, and hyphens, and reject path separators.
-
Use an exclusive lock around start and stop operations to prevent PID-file races.
-
Prefer a process supervisor or ROS lifecycle mechanism that tracks process identity securely instead of using unauthenticated PID files.
-
If identity verification fails, report the stale or mismatched state and remove the PID file only after safe ownership and path checks; never signal the process.
-