T09 · Insecure Skill Coding Practices
Error
- Location
- src/cli.ts:88
- Finding
- Shell Command Injection Through User-Controlled CLI Arguments<![CDATA[ ## Vulnerability Details **File Location**: `src/cli.ts:88-91, 109, 135, 188, 339` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```typescript // Run circle-wallet command function circleWallet(cmd: string): string { try { return execSync(`circle-wallet ${cmd}`, { encoding: 'utf-8' }); } catch (e: any) { throw new Error(e.stderr || e.message); } } ``` User-controlled values reach this function at several call sites: ```typescript circleWallet(`setup --api-key ${options.apiKey}`); ``` ```typescript const result = circleWallet(`create "${name || 'PayClaw Wallet'}"`); ``` ```typescript const result = circleWallet(`send ${address} ${amount}`); ``` ```typescript const result = circleWallet(`send ${escrow.recipient} ${escrow.amount}`); ``` ### Technical Analysis `execSync()` executes the interpolated string through a system shell. Values originating from command-line arguments—including the API key, wallet name, payment address, payment amount, and stored escrow recipient—are inserted into that string without shell-safe argument separation. The quotation marks around the wallet name do not prevent exploitation. An attacker can include a closing quotation mark followed by shell metacharacters or command substitution syntax. Unquoted values such as `address`, `amount`, and `apiKey` are directly exposed to shell parsing. The escrow release path is also vulnerable because escrow records are loaded from mutable local JSON and their `recipient` and `amount` fields are passed to the same command execution sink. ### Attack Path 1. An attacker supplies a crafted value through a CLI parameter, such as a wallet name, API key, destination address, or amount. 2. Alternatively, an attacker modifies `~/.openclaw/payclaw/escrows.json` and places shell syntax in an escrow recipient. 3. PayClaw interpolates the malicious value into a `circle-wallet` command string. 4. `execSync()` passes the resulting string ...[truncated 930 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Replace shell-string execution with argument-array execution: ```typescript import { execFileSync } from 'child_process'; function circleWallet(args: string[]): string { try { return execFileSync('circle-wallet', args, { encoding: 'utf-8', shell: false }); } catch (e: any) { throw new Error(e.stderr || e.message); } } ``` - Pass every argument as a separate array element: ```typescript circleWallet(['send', address, amount]); circleWallet(['create', name || 'PayClaw Wallet']); ``` - Validate destination addresses against the exact address format supported by the selected chain. - Require amounts to be finite, positive numbers within an explicitly defined range and serialize them canonically. - Allowlist supported chain identifiers. - Validate escrow records again after loading them from disk; never trust persisted JSON merely because the application created it. - Do not place secrets in command-line arguments. Pass the API key through protected standard input or a documented secure environment mechanism supported by `circle-wallet`. - Add automated tests containing shell metacharacters, substitutions, quotes, and newline characters to verify that inputs cannot alter the invoked executable or argument boundaries. ]]>