T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/screenshot.js:41
- Finding
- OS Command Injection Through the User-Controlled Search Query<![CDATA[ ## Vulnerability Details **File Location**: `scripts/screenshot.js:41-59` **Vulnerability Type**: OS command injection **Risk Level**: Critical ### Vulnerable Code ```javascript function exec(command, options = {}) { console.log(`[EXEC] ${command}`); try { return execSync(command, { stdio: 'pipe', encoding: 'utf8', timeout: 15000, ...options }).trim(); } catch (e) { console.log(`[WARN] Command failed: ${e.message}`); return ''; } } function searchSkills(query) { console.log(`[INFO] Searching for skills: ${query}`); const result = exec(`clawhub search ${query}`); ``` The `query` value originates from the first command-line argument: ```javascript const args = process.argv.slice(2); const query = args[0] || 'opportunity'; ``` ### Technical Analysis The application constructs a command string by directly interpolating an untrusted command-line argument: ```javascript `clawhub search ${query}` ``` That string is passed to `child_process.execSync`, which executes it through a command shell. Consequently, shell metacharacters in `query`, including command separators, substitutions, redirects, and pipelines, are interpreted by the shell rather than passed as literal ClawHub search text. The 15-second timeout only limits how long the child process may run. It does not prevent an injected command from modifying files, spawning detached processes, accessing credentials, or initiating network requests before the timeout. ### Attack Path 1. An attacker supplies or influences the first argument passed to `scripts/screenshot.js`. 2. The argument contains shell syntax in addition to an apparent search query. 3. `searchSkills` concatenates the value into `clawhub search ${query}`. 4. `exec` passes the resulting string to `execSync`. 5. The operating-system shell interprets the injected syntax. 6. The injected command executes with the same operating-system identity and environment as the Node.js proc ...[truncated 744 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Do not construct shell command strings from user input. Invoke the executable directly with an argument array and without a shell: ```javascript const { execFileSync } = require('child_process'); function searchSkills(query) { const result = execFileSync( 'clawhub', ['search', query], { stdio: 'pipe', encoding: 'utf8', timeout: 15000 } ).trim(); return result; } ``` Additional hardening should include: 1. Reject control characters and impose a reasonable maximum query length. 2. Set `shell: false` explicitly when using `spawnSync`. 3. Avoid logging raw attacker-controlled values without sanitizing terminal control characters. 4. Run the Skill under a dedicated, least-privileged account. 5. Add tests containing shell metacharacters to verify that they are passed as literal argument content. ]]>