T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:78
- Finding
- Shell Command Injection in Documented Search Integration## Vulnerability Details **File Location**: `SKILL.md`, lines 78-83 **Vulnerability Type**: OS command injection through unsafe shell command construction **Risk Level**: High **Vulnerable Code**: ```javascript const { execSync } = require('child_process'); function baiduSearch(query, count = 5) { const scriptPath = '/Users/mac/.openclaw/workspace/skills/baidu-search/baidusearch.js'; const cmd = `node "${scriptPath}" "${query}" -n ${count}`; const output = execSync(cmd, { encoding: 'utf-8' }); ``` ### Technical Analysis The documented integration places `query` and `count` directly into a command string passed to `execSync`. By default, `execSync` executes the string through a system shell. Quotation marks around `query` do not provide adequate protection because an attacker can include another quotation mark followed by shell metacharacters. For example, a query such as: ```text "; id; # ``` can terminate the intended quoted argument and append another command. The resulting command would be structurally similar to: ```sh node "/path/to/baidusearch.js" ""; id; #" -n 5 ``` The `count` argument is also interpolated without validation and can become another injection vector if an attacker can control it. The directly executable `baidusearch.js` implementation does not contain this flaw because Commander reads arguments from `process.argv`. The vulnerability is specifically present in the integration pattern recommended by `SKILL.md`. ### Attack Path 1. An application or Agent adopts the documented `baiduSearch` wrapper. 2. The attacker submits a crafted search query, or an untrusted value reaches the `count` parameter. 3. The wrapper interpolates that value into `cmd` without shell escaping or strict validation. 4. `execSync` passes the constructed string to the operating-system shell. 5. The shell interprets the injected metacharacters and executes attacker-supplied commands. 6. The inj ...[truncated 717 chars]
- Remediation
- ## Remediation Suggestions Do not construct a shell command from user-controlled values. Invoke Node.js directly with an argument array by using `execFileSync`, `spawn`, or `spawnSync` with shell execution disabled: ```javascript const { execFileSync } = require('child_process'); function baiduSearch(query, count = 5) { const scriptPath = '/Users/mac/.openclaw/workspace/skills/baidu-search/baidusearch.js'; if (typeof query !== 'string' || query.length === 0 || query.length > 500) { throw new TypeError('query must be a non-empty string of at most 500 characters'); } if (!Number.isInteger(count) || count < 1 || count > 100) { throw new RangeError('count must be an integer between 1 and 100'); } return execFileSync( process.execPath, [scriptPath, query, '-n', String(count)], { encoding: 'utf8', timeout: 30000, shell: false } ); } ``` Additional hardening should include: - Apply strict type, length, and range validation to all arguments. - Set a finite execution timeout and output-size limit. - Run the integration under a least-privileged operating-system account. - Avoid attempting to solve the issue with custom shell escaping when argument-array APIs are available. - Add automated tests containing quotation marks, semicolons, command substitutions, newlines, and other shell metacharacters.