T09 · Insecure Skill Coding Practices
Error
- Location
- index.ts:8
- Finding
- Shell Command Injection Through Untrusted Tool Arguments<![CDATA[ ## Vulnerability Details **File Location**: `index.ts:8-18` **Vulnerability Type**: OS command injection through `child_process.exec` **Risk Level**: High ### Vulnerable Code ```ts async function callBaoziMCP(toolName: string, args: any = {}) { const command = `npx -y @baozi.bet/mcp-server --tool ${toolName} --args '${JSON.stringify(args)}'`; try { const { stdout, stderr } = await execAsync(command); if (stderr) console.error('Stderr:', stderr); return JSON.parse(stdout); } catch (error) { console.error(`Error calling ${toolName}:`, error); throw error; } } ``` The same unsafe command-construction pattern is also presented in `SKILL.md:53-64`. ### Technical Analysis The function constructs a shell command by interpolating `toolName` and serialized `args` into a string passed to `child_process.exec`. The `exec` API invokes a command shell, so shell metacharacters contained in interpolated values are interpreted by that shell. Although the JSON argument is surrounded with single quotes, `JSON.stringify` does not escape characters for a POSIX shell. A single quote contained in an argument value can terminate the quoted section. Subsequent shell syntax can then introduce an additional command. Several exported handlers pass externally supplied values into this function, including: - `query` in `list-markets` - `marketId` in `get-odds`, `place-bet`, and `claim-winnings` - `wallet` in `get-portfolio` - Other properties accepted through the broadly typed `args: any` object The declared parameter schemas do not constitute shell escaping, and the implementation does not independently validate that the runtime caller enforced those schemas. ### Attack Path 1. An attacker supplies a crafted tool argument containing a single quote followed by shell syntax, for example a malicious `query`, `marketId`, or `wallet` value. 2. The corresponding handler passes the value into `callBaoziMCP`. 3. `JSON.stringify(args)` preserves the e ...[truncated 1316 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace `exec` with `execFile` or `spawn` and pass every argument as a separate array element: ```ts import { execFile } from 'child_process'; import { promisify } from 'util'; const execFileAsync = promisify(execFile); const ALLOWED_TOOLS = new Set([ 'list_markets', 'get_quote', 'build_bet_transaction_with_affiliate', 'get_portfolio', 'build_claim_transaction', ]); async function callBaoziMCP( toolName: string, args: Record<string, unknown> = {}, ) { if (!ALLOWED_TOOLS.has(toolName)) { throw new Error('Unsupported Baozi MCP tool'); } const { stdout, stderr } = await execFileAsync( process.execPath, [ require.resolve('@baozi.bet/mcp-server/dist/index.js'), '--tool', toolName, '--args', JSON.stringify(args), ], { shell: false, timeout: 30_000, maxBuffer: 1024 * 1024, }, ); if (stderr) { console.error('Baozi MCP stderr:', stderr); } return JSON.parse(stdout); } ``` 2. Prefer importing a reviewed package API directly instead of starting a subprocess. 3. Enforce strict schemas at runtime. Reject unknown properties, invalid Solana addresses, non-finite amounts, oversized strings, and malformed market identifiers. 4. Keep `toolName` restricted to a fixed internal allowlist, even if it is not currently exposed directly to users. 5. Run the skill with least privilege and without unnecessary filesystem, credential, or network access. 6. Add regression tests containing quotes, semicolons, command substitutions, newlines, and other shell metacharacters. ]]>