T09 · Insecure Skill Coding Practices
Error
- Location
- index.js:4
- Finding
- OS Command Injection Through Untrusted Skill Input## Vulnerability Details **File Location**: `index.js:4-8` **Vulnerability Type**: OS command injection **Risk Level**: Critical ### Vulnerable Code ```js async run({ input }) { const { exec } = require("child_process"); return new Promise((resolve, reject) => { exec(`hledger ${input}`, (err, stdout, stderr) => { if (err) return reject(stderr || err.message); ``` ### Technical Analysis The skill interpolates attacker-controlled `input` directly into a command passed to Node.js `child_process.exec()`. This API executes the resulting string through a system shell. Consequently, shell metacharacters and constructs within `input`, including `;`, `&&`, `|`, redirections, and command substitutions such as `$()`, are interpreted by the shell rather than treated exclusively as arguments to `hledger`. Prefixing the command with `hledger` does not restrict execution to that binary. The claim in `SKILL.md` that the skill does not permit arbitrary shell execution is inconsistent with the implementation. ### Attack Path 1. An attacker supplies crafted skill input containing a valid or attempted `hledger` argument followed by a shell operator and another command. 2. For example, the attacker submits: ```text balance; id ``` 3. The application constructs the following command: ```sh hledger balance; id ``` 4. `exec()` invokes a shell, which treats the semicolon as a command separator. 5. The shell runs both `hledger balance` and `id`. 6. More harmful commands could then read or modify files, access process-available secrets, invoke network utilities, or alter the host within the service account's permissions. ### Impact Assessment Successful exploitation provides arbitrary command execution with the operating-system privileges of the OpenClaw/Node.js process. The accessible scope may include ledger and other files readable by that account, writable application or user data, environment variables, locally available credentials, an ...[truncated 298 chars]
- Remediation
- ## Remediation Suggestions 1. Replace `child_process.exec()` with `execFile()` or `spawn()` and ensure shell execution is disabled: ```js const { execFile } = require("child_process"); execFile("hledger", validatedArgs, { shell: false }, (err, stdout, stderr) => { if (err) return reject(stderr || err.message); resolve(stdout || stderr); }); ``` 2. Do not split raw input with a simple whitespace expression because that mishandles quoting and can produce ambiguous arguments. Prefer a structured input contract in which the caller supplies an argument array. 3. Validate every argument's type and length. Enforce an allowlist of supported `hledger` subcommands and options where the intended feature set permits it. 4. Restrict file-related options such as `-f`/`--file` to explicitly authorized ledger paths. Resolve and normalize paths before comparing them against approved directories. 5. Run the skill under a dedicated least-privileged account with access only to required ledger files. Limit its environment variables, filesystem permissions, and outbound network access. 6. Add regression tests using inputs containing `;`, `&&`, `|`, backticks, `$()`, newlines, and redirection operators. Verify that these values are either rejected or passed only as literal arguments without causing secondary commands to execute. 7. Correct the security documentation in `SKILL.md` so it accurately reflects the implementation and its input restrictions.