T09 · Insecure Skill Coding Practices
Error
- Location
- src/commands/pair.ts:40
- Finding
- Shell Command Injection Through WalletConnect QR File Opening<![CDATA[ ## Vulnerability Details **File Location**: `src/commands/pair.ts:40-51` **Vulnerability Type**: OS command injection through shell interpolation **Risk Level**: High ### Vulnerable Code ```ts function openFile(filePath: string): boolean { const platform = process.platform; try { if (platform === "darwin") { execSync(`open "${filePath}"`); } else if (platform === "win32") { execSync(`start "" "${filePath}"`); } else { execSync(`xdg-open "${filePath}"`); } return true; } catch { return false; } } ``` The affected path is derived from an environment-controlled home directory: ```ts export const SESSIONS_DIR = join(process.env.HOME || "/tmp", ".agent-wallet"); ``` It reaches the vulnerable function during pairing: ```ts const qrPath = join(SESSIONS_DIR, `qr-${Date.now()}.png`); if (autoOpen) { openedBySystem = openFile(qrPath); } ``` ### Technical Analysis `execSync()` executes a command through a shell when passed a string. Although `filePath` is enclosed in double quotes, embedded quotation marks and shell metacharacters are not escaped. The QR path is constructed under `SESSIONS_DIR`, which in turn is derived from `process.env.HOME`. An attacker capable of controlling the environment of the CLI process can place shell syntax in `HOME`. That syntax becomes part of the command passed to the operating-system shell. The vulnerable path is reached automatically when stdout is a TTY and the process is not detected as an Agent environment. It is also reachable when the `--open` option forces QR opening. This functionality only needs to launch an image viewer with one path argument. Invoking a command shell exceeds the minimum privilege needed for that operation. ### Attack Path 1. The attacker controls or influences the environment used to start the Skill, including `HOME`. 2. The attacker assigns a path containing a quote and shell commands, such as a path conceptually equivalent to: ```te ...[truncated 1382 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Avoid invoking a command shell. Use an executable API that passes the file path as a discrete argument: ```ts import { spawnSync } from "child_process"; function openFile(filePath: string): boolean { let command: string; let commandArgs: string[]; if (process.platform === "darwin") { command = "open"; commandArgs = [filePath]; } else if (process.platform === "win32") { // Prefer a native Windows API or a reviewed library that does not concatenate // untrusted data into a shell command. return false; } else { command = "xdg-open"; commandArgs = [filePath]; } const result = spawnSync(command, commandArgs, { shell: false, stdio: "ignore", }); return !result.error && result.status === 0; } ``` Additional hardening should include: 1. Do not use `execSync()` or `exec()` with interpolated paths. 2. Validate that the generated QR path remains inside the intended session directory. 3. Resolve and validate `HOME` before using it as a security-sensitive storage root. 4. Reject paths containing NUL characters or other platform-invalid path components. 5. Prefer image delivery through the existing `qrPath` or media directive rather than automatically launching desktop applications. 6. Add tests using paths containing quotes, spaces, semicolons, command substitutions, and newline characters. 7. Apply the fix to both the TypeScript source and the prebuilt `dist` artifacts. ]]>