T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/process_task.js:79
- Finding
- Shell Command Injection Through Attacker-Controlled Feishu Task Content<![CDATA[ ## Vulnerability Details **File Location**: `scripts/process_task.js:79-87` **Additional Data-Flow Locations**: `scripts/process_task.js:223-229`, `scripts/process_task.js:240`, `scripts/process_task.js:256` **Vulnerability Type**: OS command injection **Risk Level**: Critical ### Vulnerable Code ```javascript async function sendFeedback(message, retryCount = 0) { try { // Escape special characters const escaped = message .replace(/\\/g, '\\\\') .replace(/"/g, '\\"') .replace(/\n/g, '\\n') .replace(/\r/g, ''); const cmd = `openclaw message send --channel feishu --message "${escaped}"`; await execAsync(cmd); ``` Attacker-controlled task content reaches this command-execution sink through feedback construction: ```javascript await sendFeedback( `📋 Task received, processing started...\n` + `Task ID: #${taskId}\n` + `Type: ${strategy.icon} ${parsed.type}\n` + `Total subtasks: ${total}\n` + `Main task: ${parsed.mainTask.substring(0, 60)}${parsed.mainTask.length > 60 ? '...' : ''}` ); ``` ```javascript const preview = subtask.replace(/^[\d\-\*•]+[\.\)]\s*/, '').substring(0, 50); ``` ```javascript await sendFeedback(`✅ Completed ${progress}% - ${preview}...`); ``` ### Technical Analysis The task processor receives Feishu task text through `process.argv[2]`. Portions of that text are subsequently included in progress messages and passed to `sendFeedback()`. `sendFeedback()` constructs a command string and executes it with `execAsync()`. Node.js `exec()` invokes a system shell. The implemented escaping only handles backslashes, double quotes, newlines, and carriage returns. It does not safely neutralize shell expansion features that remain active inside double-quoted shell strings, including command substitution using `$(...)` or backticks. Consequently, task text is interpreted partly as shell syntax rather than exclusively as the value of the `--message` argument. This is a direct command-i ...[truncated 1878 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not construct shell command strings from message content. 2. Replace `exec()` with `execFile()` or `spawn()` and pass every argument as a separate array element. This avoids shell parsing: ```javascript const { execFile } = require('child_process'); const { promisify } = require('util'); const execFileAsync = promisify(execFile); async function sendFeedback(message, retryCount = 0) { try { await execFileAsync('openclaw', [ 'message', 'send', '--channel', 'feishu', '--message', message ]); log('info', 'Feedback sent', { messageLength: message.length }); return true; } catch (error) { if (retryCount < CONFIG.maxRetries) { await sleep(CONFIG.retryDelay * (retryCount + 1)); return sendFeedback(message, retryCount + 1); } log('error', 'Feedback failed after maximum retries', { error: error.message }); return false; } } ``` 3. Apply the same shell-free invocation pattern to `scripts/listener.js:123-126`. 4. Do not rely on custom shell escaping as a security control. Correct escaping varies by shell and platform and is unnecessary when arguments are passed without a shell. 5. Add regression tests containing quotes, backticks, dollar signs, command-substitution syntax, semicolons, newlines, and platform-specific metacharacters. Verify they are transmitted literally and never executed. 6. Run the listener under a dedicated, unprivileged operating-system account with narrowly scoped filesystem and credential access to limit the consequences of any future execution flaw. ]]>