T09 · Insecure Skill Coding Practices
Error
- Location
- src/push.ts:110
- Finding
- Path Traversal Through Attacker-Controlled Agent Identifier<![CDATA[ ## Vulnerability Details **File Location**: `src/push.ts`, lines 110-125 **Vulnerability Type**: Path traversal and arbitrary file placement **Risk Level**: High ### Vulnerable Code ```typescript const id = nanoid(8); const agent = args.agent || process.env.CLAWDBOT_AGENT || 'unknown'; const session = args.session || process.env.CLAWDBOT_SESSION || 'unknown'; const filename = `${agent}-${slugify(args.title)}-${id}.md`; const filepath = join(getQueueDir('pending'), filename); const content = generateMarkdown({ ...args, id, agent, session }); ensureQueueDirs(); writeFileSync(filepath, content, 'utf-8'); ``` ### Technical Analysis The attacker-controlled `args.agent` value is inserted directly at the beginning of the filename. Unlike the title, the agent value is not passed through `slugify()` or otherwise restricted to a safe filename component. Node.js path normalization processes `../` segments contained in `agent`. Consequently, `join(getQueueDir('pending'), filename)` may resolve outside the intended `~/.arbiter/queue/pending` directory. The random plan identifier prevents an attacker from selecting the complete final filename, but it does not prevent directory traversal or arbitrary placement of a new Markdown file in another writable directory. The file content is also largely attacker-controlled. ### Attack Path 1. An attacker who can invoke `arbiter-push` supplies an agent identifier containing traversal components, for example: ```bash arbiter-push '{ "agent":"../../../../tmp/attacker-output", "title":"payload", "decisions":[{ "id":"d1", "title":"Test", "context":"Content", "options":[{"key":"a","label":"A"}] }] }' ``` 2. The application builds a filename beginning with: ```text ../../../../tmp/attacker-output-payload-<random-id>.md ``` 3. `join()` normalizes the path and resolves it outside the pending queue. 4. `writeFileSync()` creates the attacker-con ...[truncated 921 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Treat `agent` as a logical identifier, not as a path component. - Apply a strict allowlist, such as `^[A-Za-z0-9_-]{1,64}$`. - Slugify or reject unsafe values rather than silently accepting separators. - Resolve the destination and verify that it remains inside the expected directory: ```typescript import { resolve, sep } from 'node:path'; const queueDir = resolve(getQueueDir('pending')); const safeAgent = validateAgent(args.agent); const filepath = resolve(queueDir, `${safeAgent}-${slugify(args.title)}-${id}.md`); if (!filepath.startsWith(queueDir + sep)) { throw new Error('Invalid output path'); } ``` - Open the file with exclusive creation (`wx`) to avoid accidental replacement. - Add tests using `../`, absolute paths, backslashes, Unicode separators, and empty identifiers. ]]>