T09 · Insecure Skill Coding Practices
Error
- Location
- src/cli.js:13
- Finding
- OS Command Injection Through Shell-Backed OpenClaw Invocation## Vulnerability Details **File Location**: `src/cli.js`, lines 13-26 **Vulnerability Type**: OS command injection through attacker-controlled process arguments **Risk Level**: High ### Vulnerable Code ```javascript function runAgentWithThinking(message, thinkingLevel, sessionId = null) { const args = ['agent', '--thinking', thinkingLevel]; if (sessionId) { args.push('--session-id', sessionId); } args.push('--message', message); console.log(`[AutoThink] 使用 thinking=${thinkingLevel} 处理消息...\n`); return new Promise((resolve, reject) => { const proc = require('child_process').spawn('openclaw', args, { stdio: 'inherit', shell: true, env: { ...process.env } }); ``` The affected values originate from user-controlled CLI arguments and environment variables elsewhere in the same file: ```javascript let sessionId = process.env.OPENCLAW_SESSION_ID || null; ``` ```javascript case '--session-id': sessionId = args[++i]; break; ``` ```javascript const rawMessage = messageParts.join(' '); cleanedMessage = engine.cleanPrefix(rawMessage); runAgentWithThinking(cleanedMessage, thinkingMode, sessionId) ``` ### Technical Analysis The code correctly uses an argument array with `child_process.spawn()`, but then enables `shell: true`. This causes Node.js to execute the command through the operating-system shell. Attacker-controlled `message` and `sessionId` values can consequently be interpreted as shell syntax instead of being passed exclusively as literal arguments to `openclaw`. Both affected inputs are insufficiently constrained: - `message` is assembled directly from command-line input. - `sessionId` can be supplied through `--session-id` or the `OPENCLAW_SESSION_ID` environment variable. - Neither value is validated or escaped before reaching the shell. - The full inherited environment is also passed to the resulting process. Shell metachara ...[truncated 2096 chars]
- Remediation
- ## Remediation Suggestions 1. Remove shell execution and preserve direct argument passing: ```javascript const proc = require('child_process').spawn('openclaw', args, { stdio: 'inherit', shell: false, env: { ...process.env } }); ``` Omitting `shell` is also safe because its default value is `false`. 2. Validate `thinkingLevel` with a strict allowlist before process creation: ```javascript const allowedLevels = new Set(['low', 'medium', 'high']); if (!allowedLevels.has(thinkingLevel)) { throw new Error('Invalid thinking level'); } ``` 3. Validate session IDs using an allowlist appropriate to OpenClaw, such as a bounded set of letters, digits, underscores, and hyphens. Reject missing values after `--session-id`. 4. Apply reasonable length limits to messages and session IDs to reduce resource-exhaustion and malformed-input risks. 5. Do not treat shell escaping as the primary fix. Correct escaping is platform-dependent and error-prone; direct execution with `shell: false` prevents shell interpretation entirely. 6. Add regression tests that pass command separators, substitutions, redirections, quotes, spaces, and platform-specific shell characters as messages and session IDs. Verify that they are received by `openclaw` only as literal argument content and never create secondary commands. 7. Consider constructing a minimal child-process environment rather than forwarding all of `process.env`, especially when the CLI may run in privileged automation.