T09 · Insecure Skill Coding Practices
Error
- Location
- openclaw-wrapper.js:1
- Finding
- OS Command Injection Through Untrusted Search Query## Vulnerability Details **File Location**: `openclaw-wrapper.js`, lines 1-12 **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```js const { execSync } = require('child_process'); const query = process.argv[2] || ''; if (!query) { console.log('请提供搜索关键词'); process.exit(1); } try { const result = execSync(`node scripts/search.mjs "${query}" -n 5 --topic news`, { env: { ...process.env, TAVILY_API_KEY: process.env.TAVILY_API_KEY } }).toString(); ``` ### Technical Analysis The wrapper reads the search query directly from `process.argv[2]` and interpolates it into a command string passed to `child_process.execSync`. Because `execSync` executes the string through a shell, double quotes do not neutralize all shell syntax. An attacker can use command substitution or terminate the quoted argument and append another command. The child shell inherits the complete parent environment through `{ ...process.env }`, including `TAVILY_API_KEY`. Consequently, an injected process can access credentials and any other environment variables available to the wrapper. ### Attack Path 1. An attacker gains influence over the search query passed as the wrapper's second command-line argument. 2. The attacker supplies a query containing shell syntax, such as command substitution or a quote followed by a command separator. 3. The wrapper inserts that value into the command template without shell-safe argument handling. 4. `execSync` invokes a shell to interpret the resulting command. 5. The shell executes the attacker's injected command under the identity and permissions of the wrapper process. 6. The injected process can read inherited environment variables, access files available to the current user, modify accessible resources, or initiate network connections. ### Impact Assessment Successful exploitation provides arbitrary command execution with the privileges of the account ...[truncated 695 chars]
- Remediation
- ## Remediation Suggestions Replace shell-based execution with `execFileSync` or `spawnSync`, pass every argument as a separate array element, and explicitly disable shell execution. Resolve the script relative to the wrapper's own directory so behavior does not depend on the current working directory. ```js const path = require('path'); const { execFileSync } = require('child_process'); const script = path.join(__dirname, 'scripts', 'search.mjs'); const result = execFileSync( process.execPath, [script, query, '-n', '5', '--topic', 'news'], { shell: false, encoding: 'utf8', env: { PATH: process.env.PATH, TAVILY_API_KEY: process.env.TAVILY_API_KEY } } ); console.log(result); ``` Additional hardening measures: - Avoid passing the entire parent environment to child processes. Supply only variables required by Node.js and the Tavily script. - Apply a reasonable query-length limit to reduce resource abuse, while not relying on input validation as the command-injection fix. - Use structured error handling that reports failure without exposing sensitive command, environment, or response details. - Add regression tests containing quotes, semicolons, command substitutions, newlines, and other shell metacharacters, verifying that they remain literal query content.