T09 · Insecure Skill Coding Practices
- Location
reminder.js:14- Finding
Latent OS Command Injection in Push Notification Construction
- Content
View full analysis
Vulnerability Details
File Location:
reminder.js, lines 14-30
Vulnerability Type: OS command injection through unsafe shell interpolation
Risk Level: MediumVulnerable Code
javascript function sendPushNotification(messageText) { // Use the openclaw command to send a message through the Feishu channel // --channel selects the channel; --recipient current selects the current user const cmd = `openclaw message send --channel feishu --recipient current --text "${messageText.replace(/"/g, '\\"')}"`; exec(cmd, (error, stdout, stderr) => { if (error) { console.error(`Push failed: ${error.message}`); return; } if (stderr) { console.error(`Push stderr: ${stderr}`); return; } console.log(`Push succeeded: ${stdout}`); }); }Technical Analysis
sendPushNotification()incorporatesmessageTextinto a command string passed tochild_process.exec(). The function escapes only double quotation marks. This does not neutralize shell substitutions that remain active inside double-quoted strings, including:- Command substitution using
$(command) - Command substitution using backticks
- Environment-variable expansion
- Certain backslash and shell-specific expansion sequences
Event titles originate from user messages in
SOUL.mdand are incorporated into reminder messages without validation:javascript const title = parts.slice(1).join(' ');The resulting title is persisted in
MEMORY.mdand later included inmessageText. For example, a title containing$(touch /tmp/pwned)would cause the shell to executetouch /tmp/pwnedwhen the command string reachesexec().The issue is latent in the artifact as provided because
reminder.jscurrently definesloadEvents()as an empty placeholder:javascript function loadEvents() { /* ... */ }It therefore returns
undefined, and `checkRemi ...[truncated 2201 chars]- Command substitution using
- Remediation
View remediation
Remediation Suggestions
-
Replace
exec()withexecFile()orspawn()and pass every command argument separately, with shell processing explicitly disabled:javascript const { execFile } = require('child_process'); function sendPushNotification(messageText) { execFile( 'openclaw', [ 'message', 'send', '--channel', 'feishu', '--recipient', 'current', '--text', messageText ], { shell: false }, (error, stdout, stderr) => { if (error) { console.error(`Push failed: ${error.message}`); return; } if (stderr) { console.error(`Push stderr: ${stderr}`); } console.log(`Push succeeded: ${stdout}`); } ); } -
Do not attempt to make shell command construction safe through manual escaping. Correct escaping is platform- and shell-dependent and is unnecessary when argument-array APIs are used.
-
Validate event titles at ingestion:
- Enforce a reasonable maximum length.
- Reject control characters and null bytes.
- Normalize unexpected Unicode control characters.
- Treat validation as defense in depth rather than a replacement for eliminating the shell.
-
Implement
loadEvents()securely and verify that the parsed value is an array before iterating over it. Reject malformed event records rather than silently forwarding their fields into privileged operations. -
Run the skill under a dedicated, least-privileged operating-system account with restricted filesystem and credential access.
-
Add automated tests using titles containing
$(...), backticks, quotation marks, semicolons, newlines, and backslashes, and verify that none can trigger a secondary process.
-