T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/setup-cron.js:28
- Finding
- Shell Command Injection Through an Attacker-Controlled Working Directory<![CDATA[ ## Vulnerability Details **File Location**: `scripts/setup-cron.js:28-40` **Vulnerability Type**: Shell command injection in a persistent cron command **Risk Level**: High ### Vulnerable Code ```js const skillRoot = process.cwd(); function buildArgs(subcommandArgs) { const args = [...globalFlags, 'cron', ...subcommandArgs]; return args; } async function runOpenClaw(subcommandArgs) { const args = buildArgs(subcommandArgs); args.push('--token', OPENCLAW_GATEWAY_TOKEN); const result = await execFileAsync('openclaw', args, { encoding: 'utf8', env: process.env }); return result.stdout; } function commandMessage(scriptPath) { return `cd "${skillRoot}" && node ${scriptPath}`; } ``` ### Technical Analysis The script incorporates `process.cwd()` directly into a shell command stored as an OpenClaw cron message. Although the path is enclosed in double quotes, shell quoting is not safely preserved if the directory name contains a double quote, command substitution, or other shell syntax. `execFileAsync()` safely passes the OpenClaw CLI arguments without invoking a shell at setup time. However, the value supplied through `--message` is itself intended to be interpreted later as a command. Consequently, the unsafe interpolation creates a delayed command-injection vulnerability. Using `process.cwd()` also means the command is based on the directory from which the user launches the setup script, rather than a trusted path derived from the script's installed location. ### Attack Path 1. An attacker causes the project to be placed in, or invoked from, a directory with shell syntax in its name, such as a path containing: ```text skill"; touch /tmp/onlybots-compromised; # ``` 2. The user runs: ```bash node scripts/setup-cron.js ``` 3. `process.cwd()` captures the malicious directory string. 4. `commandMessage()` constructs a message equivalent to: ```bash cd ".../skill"; touch /tmp/onlybots-compromised; #" && node script ...[truncated 895 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not construct shell command strings from dynamic path values. 2. If OpenClaw supports structured execution, register the executable and arguments separately, for example: - Executable: `node` - Argument: an absolute path to the target script - Working directory: a separately specified trusted path 3. Derive the Skill root from `import.meta.url` rather than `process.cwd()`: ```js const skillRoot = resolve(__dirname, '..'); ``` 4. If a shell command is unavoidable, use a proven POSIX shell-escaping function for every dynamic value. Do not rely on double quotes alone. 5. Validate that the resolved script path remains inside the expected Skill directory. 6. Consider rejecting installation paths containing control characters or shell metacharacters as an additional defense-in-depth measure. 7. Remove and recreate any existing cron jobs after applying the fix, because previously registered messages retain the unsafe command. ]]>