T09 · Insecure Skill Coding Practices
Warning
- Location
- drift.js:71
- Finding
- Path Traversal Allows Access to JSON Files Outside the Thread Directory<![CDATA[ ## Vulnerability Details **File Location**: `drift.js:71-78` **Vulnerability Type**: Path traversal caused by insufficient validation of user-controlled thread identifiers **Risk Level**: Medium ### Vulnerable Code ```js function threadPath(id) { return path.join(DRIFT_DIR, `${id}.json`); } function loadThread(id) { const p = threadPath(id); if (!fs.existsSync(p)) return null; return JSON.parse(fs.readFileSync(p, 'utf8')); } ``` The vulnerable function is reached with command-line input by the `write`, `ask`, and `read` commands: ```js const thread = loadThread(threadId); ``` These calls occur at `drift.js:201`, `drift.js:216`, and `drift.js:330`. ### Technical Analysis The thread identifier is supplied through command-line arguments and passed to `path.join()` without validation. Although legitimate identifiers are generated as eight hexadecimal characters, the application does not enforce that format when loading a thread. An identifier containing traversal components such as `../` can cause the resulting path to resolve outside `DRIFT_DIR`. Appending `.json` limits the direct read primitive to filenames ending in that extension, but it does not ensure that the file remains within the intended storage directory. The application also does not compare the normalized or resolved path against the resolved thread-directory boundary. Consequently, a local caller can make the process read any accessible JSON file whose path can be expressed relative to `DRIFT_DIR`. The `write` and `ask` operations subsequently call: ```js function saveThread(thread) { ensureDir(); fs.writeFileSync(threadPath(thread.id), JSON.stringify(thread, null, 2)); } ``` The save destination is derived from the loaded object's `id` property. If an attacker can arrange for a traversed JSON file to contain a compatible thread object with a malicious `id`, mutation commands may also write outside the thread directory. This write scenario requires control over, o ...[truncated 1847 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Enforce the format used by generated identifiers before any filesystem operation: ```js function validateThreadId(id) { if (!/^[0-9a-f]{8}$/.test(id)) { throw new Error('Invalid thread identifier'); } } ``` 2. Resolve the candidate path and verify that it remains directly inside the configured thread directory: ```js function threadPath(id) { validateThreadId(id); const base = path.resolve(DRIFT_DIR); const candidate = path.resolve(base, `${id}.json`); if (path.dirname(candidate) !== base) { throw new Error('Thread path escapes storage directory'); } return candidate; } ``` 3. Validate every parsed thread object against a strict schema. Require its `id` to match the requested identifier and the expected hexadecimal format before passing it to `saveThread()`. 4. Do not trust the `id` embedded in a loaded file when selecting the save destination. Preserve the validated identifier used to open the file and pass it explicitly to the save operation. 5. Where supported, reject symbolic links or use filesystem operations that prevent following attacker-controlled links. Path containment checks alone do not fully address symlink-based boundary violations. 6. Add regression tests for identifiers containing `../`, absolute paths, mixed path separators, encoded separators, malformed IDs, symlinks, and loaded objects whose embedded `id` differs from the requested ID. ]]>