T09 · Insecure Skill Coding Practices
Error
- Location
- index.js:100
- Finding
- Shell Command Injection Through the HOME Environment Variable## Vulnerability Details **File Location**: `index.js`, lines 100–101 **Vulnerability Type**: OS command injection through unquoted shell interpolation **Risk Level**: High ### Vulnerable Code ```javascript const wrapperPath = path.join(process.env.HOME, '.openclaw', 'workspace', 'skills', 'feishu-evolver-wrapper'); const output = execSync(`cd ${wrapperPath} && node lifecycle.js status 2>/dev/null || echo "unknown"`, { ``` ### Technical Analysis The value of `process.env.HOME` is used to construct `wrapperPath`, which is then interpolated directly into a command passed to `execSync()`. By default, `execSync()` executes the string through a system shell. Because the path is neither safely quoted nor passed as a separate argument, shell metacharacters contained in `HOME` are interpreted as command syntax. An attacker who can influence the process environment could set `HOME` to a value such as `/tmp/x; attacker_command; #`. When `getWrapperStatus()` executes, the resulting shell command would run `attacker_command` under the identity of the Node.js process. The vulnerable function is reached through `checkHealth()`. It is therefore also indirectly reachable through `getFailedAgents()` and every iteration of `startMonitoring()`. ### Attack Path 1. The attacker gains control over, or can influence, the environment variables supplied when the Agent process or skill runner starts. 2. The attacker sets `HOME` to a value containing shell control characters and an injected command, for example `/tmp/x; attacker_command; #`. 3. A caller invokes `getWrapperStatus()`, `checkHealth()`, `getFailedAgents()`, or `startMonitoring()`. 4. The skill constructs `wrapperPath` from the malicious `HOME` value. 5. `execSync()` passes the interpolated command string to the system shell. 6. The shell interprets the injected metacharacters and executes the attacker-supplied command. Exploitation requires the attacker to influence the runtime ...[truncated 689 chars]
- Remediation
- ## Remediation Suggestions Avoid invoking a shell for this operation. Execute Node.js directly and provide the working directory through the process API: ```javascript const { execFileSync } = require('child_process'); const home = process.env.HOME; if (!home || !path.isAbsolute(home)) { return { status: 'unknown', pid: null }; } const wrapperPath = path.resolve( home, '.openclaw', 'workspace', 'skills', 'feishu-evolver-wrapper' ); const output = execFileSync( process.execPath, ['lifecycle.js', 'status'], { cwd: wrapperPath, encoding: 'utf-8', timeout: 5000, stdio: ['ignore', 'pipe', 'ignore'] } ); ``` Additional hardening measures: 1. Resolve and validate the directory against an explicitly trusted OpenClaw root rather than relying blindly on `HOME`. 2. Confirm that the resolved wrapper path remains inside the expected workspace before execution. 3. Handle command failure in JavaScript instead of using shell operators such as `||` and shell redirection. 4. Use `execFileSync()` or `spawnSync()` with argument arrays for the other fixed commands in the file as a defense-in-depth measure. 5. Run the skill under a least-privileged account with only the filesystem permissions required for health monitoring. 6. Add a regression test using `HOME` values containing spaces and shell metacharacters, verifying that no additional command can execute.