T09 · Insecure Skill Coding Practices
- Location
src/cli.js:13- Finding
OS Command Injection Through Shell-Backed OpenClaw Invocation
- Content
View full analysis
Vulnerability Details
File Location:
src/cli.js, lines 13-26
Vulnerability Type: OS command injection through attacker-controlled process arguments
Risk Level: HighVulnerable 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 enablesshell: true. This causes Node.js to execute the command through the operating-system shell. Attacker-controlledmessageandsessionIdvalues can consequently be interpreted as shell syntax instead of being passed exclusively as literal arguments toopenclaw.Both affected inputs are insufficiently constrained:
messageis assembled directly from command-line input.sessionIdcan be supplied through--session-idor theOPENCLAW_SESSION_IDenvironment 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
View remediation
Remediation Suggestions
-
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
shellis also safe because its default value isfalse. -
Validate
thinkingLevelwith a strict allowlist before process creation:javascript const allowedLevels = new Set(['low', 'medium', 'high']); if (!allowedLevels.has(thinkingLevel)) { throw new Error('Invalid thinking level'); } -
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. -
Apply reasonable length limits to messages and session IDs to reduce resource-exhaustion and malformed-input risks.
-
Do not treat shell escaping as the primary fix. Correct escaping is platform-dependent and error-prone; direct execution with
shell: falseprevents shell interpretation entirely. -
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
openclawonly as literal argument content and never create secondary commands. -
Consider constructing a minimal child-process environment rather than forwarding all of
process.env, especially when the CLI may run in privileged automation.
-