T09 · Insecure Skill Coding Practices
Error
- Location
- sms-webhook-server.js:44
- Finding
- Arbitrary Command Execution Through Unsafely Constructed OpenClaw CLI Command<![CDATA[ ## Vulnerability Details **File Location**: `sms-webhook-server.js`, lines 44-51 **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```js const msg = `📱 SMS from ${data.contact || 'Unknown'}: ${data.preview || data.message || '(no content)'}`; try { const cmd = `openclaw message send -t "${NOTIFICATION_TARGET}" --channel ${NOTIFICATION_CHANNEL} -m "${msg.replace(/"/g, '\\"').replace(/\n/g, ' ')}"`; execSync(cmd, { timeout: 15000, stdio: 'pipe' }); console.log('✅ Forwarded to', NOTIFICATION_CHANNEL); } catch (e) { ``` ### Technical Analysis The webhook constructs a shell command by interpolating values into a single command string and passes that string to `child_process.execSync`. The message includes the webhook-controlled fields `data.contact`, `data.preview`, or `data.message`. The code only escapes double quotes and replaces newline characters. This does not prevent shell evaluation inside double-quoted strings. Shell command substitutions such as `$(command)` and backtick substitutions can still be executed. Shell metacharacters in the unquoted `SMS_NOTIFICATION_CHANNEL` configuration value create an additional injection vector. The vulnerable function is reached by the unauthenticated `POST /sms-inbound` endpoint. The browser observer also automatically copies incoming SMS contact names and previews into this endpoint, making malicious SMS content a practical source of attacker-controlled input. ### Attack Path 1. The victim starts the webhook server with `SMS_NOTIFICATION_TARGET` configured, enabling forwarding. 2. An attacker sends the victim an SMS whose visible message preview contains shell command-substitution syntax, such as `$(attacker_command)`. 3. `sms-observer.js` detects the changed incoming message preview and submits it in JSON to `http://127.0.0.1:19888/sms-inbound`. 4. The webhook server parses the JSON without validating the contact or message fields. 5. `forwardToOpenClaw` inse ...[truncated 1271 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Eliminate shell command construction.** Use `execFileSync` or `spawnSync` with an argument array and explicitly disable shell execution: ```js const { spawnSync } = require('child_process'); const result = spawnSync( 'openclaw', [ 'message', 'send', '-t', NOTIFICATION_TARGET, '--channel', NOTIFICATION_CHANNEL, '-m', msg ], { shell: false, timeout: 15000, stdio: 'pipe' } ); if (result.error || result.status !== 0) { throw result.error || new Error(`openclaw exited with status ${result.status}`); } ``` 2. **Validate configuration values.** Restrict `SMS_NOTIFICATION_CHANNEL` to a fixed allowlist of supported channel names. Validate `SMS_NOTIFICATION_TARGET` against the expected syntax for the selected channel. 3. **Validate webhook data.** Require `contact`, `preview`, and `message` to be strings; impose conservative length limits; reject unexpected fields and malformed payloads. Validation is defense in depth and must not replace removal of shell execution. 4. **Authenticate the webhook.** Generate a high-entropy shared secret and require it in an authorization header for `POST /sms-inbound`. Compare it using a timing-safe method. 5. **Restrict browser access.** Replace `Access-Control-Allow-Origin: *` with an explicit trusted origin policy where browser behavior permits it, and reject unexpected `Origin` values. 6. **Limit request bodies.** Stop reading and reject the request once a small maximum payload size is exceeded to reduce denial-of-service exposure. 7. **Apply least privilege.** Run the webhook under a dedicated, restricted account with minimal filesystem access, no administrative privileges, and only the environment variables required for forwarding. 8. **Add regression tests.** Verify that payloads containing `$()`, backticks, quotes, semicolons, pipes, redirections, and newlin ...[truncated 83 chars]
