Security audit

Stdio Skill

Security checks for vulnerabilities and agentic risk

Overview

This skill is a local file dropbox, but its implementation can store and operate on files outside the documented workspace boundary.

Review this before installing. The skill appears to be a simple local file-transfer helper rather than malware, but its filesystem boundary is not reliable as written. Use it only in a contained environment, avoid placing sensitive files near its boxes, and prefer a fixed version that stores data under the intended project directory, rejects symlinks, limits file/message size, and clearly warns before overwrites or deletes.

A.I.G

Vulnerability Patterns

Unauthorized Access and Privilege EscalationObtains permissions beyond the task's legitimate needs
Insecure Skill Coding PracticesFinds exploitable flaws such as hardcoded secrets or command injection
Skill Instruction HijackingAlters the agent's session goals or safety constraints when the skill loads
Agent Memory PoisoningWrites attacker-controlled rules into memory that affect later sessions
Remote Payload Retrieval and ExecutionFetches external code whose behavior can change after review

Findings (3)

T05 · Unauthorized Access and Privilege Escalation

Error

Location: scripts/server.js:20
Finding: Filesystem storage is created outside the documented project boundary<![CDATA[ ## Vulnerability Details **File Location**: `scripts/server.js`, lines 20–28 **Vulnerability Type**: Incorrect filesystem root resolution **Risk Level**: High ### Vulnerable Code ```js const ROOT = path.resolve(__dirname, '..', '..', '..'); // repo root const BASE = path.join(ROOT, 'stdio'); const BOXES = { inbox: path.join(BASE, 'inbox'), outbox: path.join(BASE, 'outbox'), tmp: path.join(BASE, 'tmp'), }; for (const p of Object.values(BOXES)) fs.mkdirSync(p, { recursive: true }); ``` ### Technical Analysis The server claims that `ROOT` is the repository root, but it traverses three parent directories from `scripts/server.js`. Under the audited project layout, the script is located at: ```text /tmp/clawhub-codex-scan-v57axzqee6rxhth9q50v4f5be58e4144-8BV93k/artifact/scripts/server.js ``` Resolving three parent components from `artifact/scripts` produces `/tmp`, rather than the `artifact` project directory. Consequently, the server creates and operates on these predictable shared paths: ```text /tmp/stdio/inbox /tmp/stdio/outbox /tmp/stdio/tmp ``` This behavior contradicts the workspace-relative storage model documented in `SKILL.md`. It also eliminates project-level isolation: other instances, projects, or processes running with sufficient permissions can interact with the same file boxes. The directories are created without explicit restrictive permission modes. Their effective permissions therefore depend on the process umask and any pre-existing `/tmp/stdio` directory. ### Attack Path 1. The MCP server starts and resolves `ROOT` to `/tmp`. 2. It creates or reuses `/tmp/stdio/inbox`, `/tmp/stdio/outbox`, and `/tmp/stdio/tmp`. 3. Another local process running as the same account, or otherwise having access to those directories, places a file in one of the shared boxes. 4. The Skill lists or reads that file as if it belonged to the current project. 5. Through the exposed tools, the file can be read, overwritten, moved, or deleted. 6. Co ...[truncated 776 chars]
Remediation: <![CDATA[ ## Remediation Suggestions Resolve the storage root from the actual project directory. Given the audited layout, use: ```js const ROOT = path.resolve(__dirname, '..'); const BASE = path.join(ROOT, 'stdio'); ``` Additional hardening should include: 1. Create project-specific storage directories rather than a predictable globally shared directory. 2. Set restrictive permissions explicitly: ```js for (const p of Object.values(BOXES)) { fs.mkdirSync(p, { recursive: true, mode: 0o700 }); fs.chmodSync(p, 0o700); } ``` 3. Refuse to use a pre-existing base directory unless its owner and permissions are trusted. 4. Verify at startup that the canonical `BASE` path is inside the expected canonical project root. 5. Avoid returning absolute host paths through `stdio_paths` unless disclosure is operationally necessary. 6. Add a regression test asserting that every box path remains beneath the project root. ]]>

T05 · Unauthorized Access and Privilege Escalation

Error

Location: scripts/server.js:37
Finding: Symbolic links bypass filesystem confinement for file reads and writes<![CDATA[ ## Vulnerability Details **File Location**: `scripts/server.js`, lines 37–71 **Vulnerability Type**: Symbolic-link traversal and time-of-check/time-of-use weakness **Risk Level**: High ### Vulnerable Code ```js function resolveInBox(box, name) { if (!BOXES[box]) throw new Error(`unknown box: ${box}`); const n = safeName(name); const p = path.join(BOXES[box], n); // Ensure still within box const rel = path.relative(BOXES[box], p); if (rel.startsWith('..') || path.isAbsolute(rel)) throw new Error('path traversal'); return p; } ``` The returned path is subsequently opened with APIs that follow symbolic links: ```js function readFileBase64(box, name) { const p = resolveInBox(box, name); const buf = fs.readFileSync(p); return { name, box, bytes: buf.length, contentBase64: buf.toString('base64') }; } function writeFileBase64(box, name, contentBase64, overwrite = false) { const p = resolveInBox(box, name); const exists = fs.existsSync(p); if (exists && !overwrite) throw new Error('file exists (set overwrite=true to replace)'); const buf = Buffer.from(String(contentBase64 || ''), 'base64'); fs.writeFileSync(p, buf); return { name, box, bytes: buf.length, overwritten: exists }; } ``` ### Technical Analysis `resolveInBox` checks only whether the lexical pathname is beneath the selected box. It does not inspect the final directory entry with `lstat`, reject symbolic links, or verify the canonical target with `realpath`. A path such as `/tmp/stdio/inbox/secret` is lexically inside the inbox even when `secret` is a symbolic link to a file outside that directory. Both `fs.readFileSync` and `fs.writeFileSync` follow symbolic links by default. The `existsSync` check before writing also introduces a time-of-check/time-of-use window. A local process able to modify the box can change the destination entry after the existence check but before `writeFileSync` opens it. Exploitation requires the attacker to be able to create or repl ...[truncated 1964 chars]
Remediation: <![CDATA[ ## Remediation Suggestions Do not rely only on lexical path checks. Apply defense in depth: 1. Reject symbolic links with `fs.lstatSync` before reading, deleting, moving, or overwriting an existing entry. 2. Canonicalize the box directory with `fs.realpathSync` and verify that an existing target's canonical path remains beneath it. 3. Open files through file descriptors using `O_NOFOLLOW` where supported: ```js const flags = fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW; const fd = fs.openSync(targetPath, flags); try { const stat = fs.fstatSync(fd); if (!stat.isFile()) throw new Error('target must be a regular file'); // Read through the validated descriptor. } finally { fs.closeSync(fd); } ``` 4. For new files, use exclusive creation (`O_CREAT | O_EXCL | O_NOFOLLOW`) with restrictive permissions. 5. For replacement operations, write to a securely created temporary regular file in the same private directory, validate it, and atomically rename it into place. 6. Confirm with `fstat` that opened objects are regular files. 7. Ensure box directories are owned by the expected account and are not writable by untrusted users. 8. Remove the separate shared-directory condition by correcting the project root. 9. Add tests involving final-component symlinks, broken symlinks, race attempts, and links to files outside the box. ]]>

T09 · Insecure Skill Coding Practices

Warning

Location: scripts/server.js:57
Finding: Unbounded protocol and file buffering permits denial of service<![CDATA[ ## Vulnerability Details **File Location**: `scripts/server.js`, lines 57–70 and 347–349 **Vulnerability Type**: Uncontrolled resource consumption **Risk Level**: Medium ### Vulnerable Code Entire files and decoded base64 payloads are held in memory: ```js function readFileBase64(box, name) { const p = resolveInBox(box, name); const buf = fs.readFileSync(p); return { name, box, bytes: buf.length, contentBase64: buf.toString('base64') }; } function writeFileBase64(box, name, contentBase64, overwrite = false) { const p = resolveInBox(box, name); const exists = fs.existsSync(p); if (exists && !overwrite) throw new Error('file exists (set overwrite=true to replace)'); const buf = Buffer.from(String(contentBase64 || ''), 'base64'); fs.writeFileSync(p, buf); return { name, box, bytes: buf.length, overwritten: exists }; } ``` Incoming protocol data is accumulated without a size limit: ```js process.stdin.on('data', (chunk) => { buf = Buffer.concat([buf, chunk]); parseMessages(); }); ``` The declared LSP body length is also accepted without an upper bound: ```js const len = parseInt(m[1], 10); const total = headerEnd + sepLen + len; if (buf.length < total) return; ``` ### Technical Analysis The server imposes no maximum on: - Buffered stdin data. - LSP `Content-Length`. - NDJSON line length. - Base64 input length. - Decoded output size. - Size of a file returned by `stdio_read`. Repeated `Buffer.concat` operations allocate a new buffer and copy existing data whenever another stdin chunk arrives. For a large incomplete request, this can produce substantial allocation and copying overhead. A declared but never completed large `Content-Length` causes the process to retain received data indefinitely while waiting for the remaining body. `stdio_read` loads an entire file and then creates an additional base64 representation, which is approximately one-third larger than the binary input. `stdio_write` similarly materializes the JSON ...[truncated 1584 chars]
Remediation: <![CDATA[ ## Remediation Suggestions 1. Define explicit limits for protocol messages, encoded payloads, decoded files, and readable files. 2. Reject excessive `Content-Length` values before waiting for or allocating the body: ```js const MAX_MESSAGE_BYTES = 8 * 1024 * 1024; const len = Number.parseInt(m[1], 10); if (!Number.isSafeInteger(len) || len < 0 || len > MAX_MESSAGE_BYTES) { throw new Error('message exceeds maximum size'); } ``` 3. Terminate or reset the connection if buffered data exceeds the configured maximum, including incomplete NDJSON records. 4. Validate base64 input length before decoding and reject decoded data larger than the file limit. 5. Check file size with `fstat` before reading, while recognizing that descriptor-based streaming is safer than a separate pathname check. 6. Stream large reads and writes instead of using whole-file buffers. If MCP response constraints require base64 text, impose a conservative maximum file size. 7. Replace repeated `Buffer.concat` with a bounded framing parser that retains chunks without repeatedly copying all accumulated data. 8. Use asynchronous filesystem APIs to avoid blocking the event loop. 9. Apply operating-system resource limits and supervision so that a single failed process cannot consume excessive host memory. 10. Add tests for oversized headers, incomplete bodies, long NDJSON records, large base64 payloads, and oversized inbox files. ]]>

SkillSpector

Vulnerability Patterns

Excessive AgencyUnrestricted Tool Access, Autonomous Decision Making, Scope Creep
MCP Tool PoisoningHidden Instructions, Unicode Deception, Parameter Description Injection
Prompt InjectionInstruction Override, Hidden Instructions, Exfiltration Commands
Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
Privilege EscalationExcessive Permissions, Sudo/Root Execution, Credential Access

Findings (4)

Missing User Warnings

Medium

Confidence: 92% confidence
Finding: This code implements direct file writes, moves with overwrite, and deletions via stdio_write, stdio_move, and stdio_delete, including irreversible actions like fs.unlinkSync, but provides no confirmation prompt, warning message, or user-facing disclosure at execution time. Although the tool descriptions state what the operations do, they do not warn users about destructive effects such as replacement or deletion of existing files.

Description-Behavior Mismatch

Medium

Confidence: 92% confidence
Finding: The skill exposes absolute on-disk paths, including the resolved repository root, through the stdio_paths tool. Even without arbitrary path access, this leaks environment structure that can aid follow-on targeting, reduce isolation assumptions, and reveal more host context than is necessary for an inbox/outbox bridge.

Context-Inappropriate Capability

Medium

Confidence: 91% confidence
Finding: The stdio_paths capability provides path-discovery unrelated to the core stated purpose of moving files through inbox/outbox/tmp. Unnecessary discovery features expand attack surface by giving clients environmental reconnaissance data that can be combined with other weaknesses or operational mistakes.

Missing User Warnings

Low

Confidence: 88% confidence
Finding: The markdown instructs the agent to use `stdio_move` to claim files by moving them to `tmp`, which changes file location and could affect a user's expected workflow. Although the document describes the inbox/tmp/outbox model, it does not explicitly warn users that inputs may be relocated during processing or that outputs may be written to disk.

Static analysis

No suspicious patterns detected.