T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/setup-crons.js:939
- Finding
- Stored Command Injection Through Custom Podcast Style Names<![CDATA[ ## Vulnerability Details **File Location**: `scripts/setup-crons.js`, lines 939-944 and 985-993 **Vulnerability Type**: Stored command injection caused by unsafe shell-command construction **Risk Level**: High ### Vulnerable Code ```javascript const styleArg = style.name.replace(/'/g, "\\'"); return [ `openclaw cron add "${jobName}" \\`, ` --schedule "${cronTime}" \\`, ` --command "node ${scriptPath} --style '${styleArg}' --time-of-day ${timeOfDay}"`, ].join('\n'); ``` The same unsafe construction is used when the wizard directly registers the cron job: ```javascript const styleArg = job.style.name; const scriptPath = path.join(skillPath, 'scripts', 'generate-episode.js'); const cmdStr = `node ${scriptPath} --style '${styleArg}' --time-of-day ${job.timeOfDay}`; try { execFileSync('openclaw', [ 'cron', 'add', jobName, '--schedule', job.cronTime, '--command', cmdStr, ], { stdio: 'inherit' }); ``` ### Technical Analysis Custom style names originate from user-controlled input and are inserted into a command string that is stored as an OpenClaw cron command. The attempted escaping in `buildCronCommand()` is not valid POSIX shell escaping. A backslash does not escape a single quote while inside a single-quoted shell string. For example, a style name such as: ```text x'; touch /tmp/openclaw-podcast-pwned; # ``` would produce a stored command resembling: ```sh node /path/generate-episode.js --style 'x'; touch /tmp/openclaw-podcast-pwned; #' --time-of-day morning ``` Although `execFileSync()` invokes the `openclaw` executable without a shell, the vulnerable value is placed inside the `--command` argument. The command is intended to be interpreted later by the cron execution environment. Consequently, avoiding a shell at registration time does not prevent injection when the stored cron command runs. The script path is also inserted without quoting, which can cause additional command parsing problems when the installat ...[truncated 1556 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not construct a shell command by concatenating user-controlled values. 2. Prefer a cron API that accepts an executable and argument array separately, for example: ```javascript { executable: process.execPath, args: [ scriptPath, '--style', style.name, '--time-of-day', job.timeOfDay, ], } ``` 3. If OpenClaw only accepts a command string, use a well-reviewed shell-quoting implementation that correctly serializes every argument. Do not implement quoting with simple string replacement. 4. Apply strict validation to custom style names. For example, allow only letters, numbers, spaces, hyphens, underscores, and a limited set of punctuation: ```javascript if (!/^[A-Za-z0-9 _&-]{1,80}$/.test(style.name)) { throw new Error('Style name contains unsupported characters'); } ``` 5. Quote or serialize `scriptPath` using the same safe mechanism. 6. Validate `timeOfDay` against the existing fixed allowlist before command construction. 7. Add automated tests using hostile values containing single quotes, double quotes, semicolons, command substitution, newlines, backticks, and shell redirection. 8. Display the exact stored command and require explicit confirmation when custom styles are scheduled. ]]>