T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/queue_and_return.py:22
- Finding
- Unverified Persisted PIDs Can Cause Termination of Unrelated Processes<![CDATA[ ## Vulnerability Details **File Location**: `scripts/queue_and_return.py`, lines 22–44 **Vulnerability Type**: Trust of mutable process identifiers and unsafe process termination **Risk Level**: Medium ### Vulnerable Code ```python def _active_workers(handoff_dir: pathlib.Path, stale_seconds: int) -> tuple[int, list[dict]]: now = int(time.time()) active: list[dict] = [] for f in sorted(handoff_dir.glob('*.json')): try: row = json.loads(f.read_text()) except Exception: continue pid = row.get('worker_pid') started = int(row.get('worker_started_at_s') or 0) if not isinstance(pid, int) or pid <= 0: continue alive = _is_pid_alive(pid) stale = bool(started and (now - started > stale_seconds)) if stale and alive: try: os.kill(pid, signal.SIGTERM) time.sleep(0.2) if _is_pid_alive(pid): os.kill(pid, signal.SIGKILL) except OSError: pass alive = _is_pid_alive(pid) row['orphan_cleanup'] = {'stale_seconds': stale_seconds, 'cleaned_at_s': now, 'terminated': not alive} f.write_text(json.dumps(row, ensure_ascii=False, indent=2) + '\n') if alive: active.append({'pid': pid, 'request_id': row.get('request_id'), 'file': str(f)}) return len(active), active ``` ### Technical Analysis The background-worker cleanup logic treats every JSON file in the configurable handoff directory as authoritative process state. It validates only that `worker_pid` is a positive integer and that a process with that PID is alive. It does not verify: - That the PID belongs to a process originally launched by this Skill. - That the executable is Python or the expected `run_image_queue.py` worker. - That the process start time matches the recorded worker start time. - That the process belongs to the expected queue or ...[truncated 1967 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Verify process identity before sending signals** - Read `/proc/<pid>/cmdline` and verify that it identifies the expected Python interpreter and `run_image_queue.py`. - Verify that command-line arguments contain the expected queue directory and request ID. - Compare the process start time with a start time recorded when the worker was launched. 2. **Use stronger worker identity** - Generate an unpredictable worker token when launching a worker. - Pass the token to the worker and record it in a protected state file. - Require all identity checks to succeed before treating a process as owned by the Skill. 3. **Protect state files** - Create queue and handoff directories with owner-only permissions where possible. - Reject handoff files not owned by the expected user. - Use atomic file creation and updates. - Do not trust state files from shared or attacker-writable directories. 4. **Handle PID reuse** - Store both the PID and operating-system process start time. - Treat a mismatch as a stale record that must be removed without signaling the current process. 5. **Reduce termination aggressiveness** - Avoid automatically escalating to `SIGKILL` after only 0.2 seconds. - Provide a reasonable shutdown timeout. - If worker ownership cannot be conclusively verified, remove or quarantine the record without signaling the process. 6. **Run with minimum privileges** - Never run the queue launcher as root or under an account that owns unrelated critical services. ]]>