T09 · Insecure Skill Coding Practices
Note
- Location
- scripts/voice_journal_ctl.js:66
- Finding
- Unverified Stale PID Can Terminate an Unrelated Process<![CDATA[ ## Vulnerability Details **File Location**: `scripts/voice_journal_ctl.js`, lines 66–70 and 143–149 **Vulnerability Type**: Improper process identity validation **Risk Level**: Low ### Vulnerable Code ```javascript function readPid() { if (!fs.existsSync(paths.pid)) return null; const raw = fs.readFileSync(paths.pid, "utf8").trim(); const pid = Number(raw); return Number.isInteger(pid) ? pid : null; } ``` ```javascript async function end() { const pid = readPid(); if (!pid || !isPidRunning(pid)) { console.log("Voice journal is not running."); return; } process.kill(pid, "SIGTERM"); ``` The process-liveness check used before termination is: ```javascript function isPidRunning(pid) { if (!Number.isInteger(pid) || pid <= 1) return false; try { process.kill(pid, 0); return true; } catch (_) { return false; } } ``` ### Technical Analysis The controller treats the numeric value in `.data/daemon.pid` as sufficient proof of the daemon's identity. The `isPidRunning()` function establishes only that a process currently owns the PID; it does not establish that the process is the voice journal daemon started by this Skill. If the daemon exits abnormally, such as through `SIGKILL`, a system crash, or another path that bypasses its shutdown cleanup, its PID file can remain in place. Operating systems eventually reuse process identifiers. Once the stale PID is assigned to another same-user process, the `end` command considers that unrelated process to be the active daemon and sends it `SIGTERM`. The `.data` directory is hardened to mode `0700` and files to `0600`, which limits cross-user tampering. However, those permissions do not prevent natural PID reuse or manipulation by another process operating under the same user account. ### Attack Path 1. Start the voice journal so that the daemon writes its PID to `.data/daemon.pid`. 2. Cause the daemon to exit without executing its normal shutdown cleanup, for examp ...[truncated 1051 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Use an authenticated daemon-control mechanism rather than relying solely on a reusable numeric PID: 1. **Preferred approach: authenticated local IPC** - Create a Unix-domain socket inside the permission-restricted `.data` directory. - Generate a cryptographically random instance token when starting the daemon. - Require the controller to present that token over the socket. - Let the daemon perform its own authenticated shutdown after receiving the request. 2. **Use exclusive process locking** - Hold an exclusive lock for the daemon's lifetime. - Treat a PID file without a corresponding live lock as stale. - Remove stale state before starting or stopping a daemon. 3. **Validate process identity before signaling** - Record additional startup identity information, such as the daemon's executable path, script path, process start time, and a random instance identifier. - Before sending a signal, verify the target's command line and start time using an operating-system-specific process interface. - Do not signal the process if any identity attribute differs. 4. **Improve stale-state cleanup** - On startup and before shutdown, detect PID files that do not correspond to a verified daemon instance and remove them safely. - Continue removing the PID file during graceful shutdown, while recognizing that cleanup alone cannot handle `SIGKILL` or system crashes. 5. **Fail closed** - If process identity cannot be established reliably, report stale or unverifiable daemon state and require manual cleanup rather than sending `SIGTERM`. ]]>