T09 · Insecure Skill Coding Practices
Error
- Location
- src/monitor.js:49
- Finding
- OS Command Injection in DingTalk Notification Handling<![CDATA[ ## Vulnerability Details **File Location**: `src/monitor.js`, lines 49–60 **Vulnerability Type**: OS command injection through shell interpolation **Risk Level**: Critical ### Vulnerable Code ```js class Notifier { static async send(config, message) { if (config.type === 'dingtalk') { try { const { execSync } = require('child_process'); execSync(`openclaw message send --target "${config.target}" --message "${message.replace(/"/g, '\\"')}"`, { stdio: 'pipe' }); console.log('✅ 消息已发送'); } catch (e) { console.error('❌ 发送失败:', e.message); console.log(message); } } else { console.log(message); } } } ``` ### Technical Analysis The notification target and generated report are interpolated into a command string passed to `child_process.execSync()`. This API executes the string through a system shell. Escaping only double quotation marks in `message` is insufficient. Shell constructs such as command substitutions using `$(...)` or backticks remain active inside double-quoted shell arguments. The `config.target` value is not escaped at all. The report includes remotely sourced notice titles, areas, and dates extracted from the monitored page in `src/monitor.js` lines 123–134. Consequently, this vulnerability crosses two separate trust boundaries: 1. A malicious or compromised configuration can inject commands through `notify.target`. 2. A malicious or compromised monitored website can inject shell syntax into scraped notice content. For example, a matching notice title containing `$(malicious-command)` would be included in the generated report and evaluated by the shell when the notification is sent. ### Attack Path 1. An attacker controls or compromises a website monitored by the Skill. 2. The attacker publishes a table row whose title contains one of the configured keywords and a shell substitu ...[truncated 1306 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Do not construct a shell command from notification data. Invoke OpenClaw directly with an argument array and disable shell processing: ```js const { execFileSync } = require('child_process'); execFileSync( 'openclaw', [ 'message', 'send', '--target', config.target, '--message', message ], { stdio: 'pipe', shell: false } ); ``` Additional hardening should include: 1. Validate `config.target` against the exact DingTalk identifier format expected by OpenClaw. 2. Reject targets containing control characters, shell metacharacters, or unexpected whitespace. 3. Apply reasonable length limits to titles, areas, dates, task names, and complete reports. 4. Treat all scraped page content as untrusted, even when monitoring a government domain. 5. Prefer a direct OpenClaw API or SDK over invoking a command-line process. 6. Add regression tests containing `$()`, backticks, quotes, newlines, semicolons, and other shell metacharacters. 7. Run the Skill under a dedicated low-privilege operating-system account to limit damage if another injection flaw is introduced. ]]>