T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/communicator.js:89
- Finding
- OS Command Injection Through Unsanitized Session Key<![CDATA[ ## Vulnerability Details **File Location**: `scripts/communicator.js:89-93` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```javascript async function protectSession(sessionKey) { const result = await exec({ command: `openclaw sessions cleanup --active-key "${sessionKey}" --enforce` }); return result; } ``` The same unsafe command construction is recommended in `SKILL.md:151-153` and `SKILL.md:176-179`. ### Technical Analysis The `protectSession` function directly interpolates the caller-supplied `sessionKey` into a command string passed to `exec`. No validation, escaping, or argument separation is applied. Wrapping the value in double quotes does not make shell interpolation safe. An attacker-controlled value can terminate the quoted argument and introduce shell metacharacters or additional commands. Depending on the shell used by the OpenClaw `exec` tool, command substitution syntax may also be evaluated inside double quotes. The flaw becomes exploitable whenever an untrusted or insufficiently validated value can reach `protectSession`. The function is exported by the module, so any consumer of this helper can invoke it directly. ### Attack Path 1. An attacker obtains influence over a value passed as `sessionKey`, such as through an upstream message, tool result, integration, or direct invocation of the exported function. 2. The attacker supplies a session-key value containing quote-breaking syntax and a shell command, conceptually: ```text valid-prefix"; attacker-command; # ``` 3. `protectSession` embeds that value into the command: ```text openclaw sessions cleanup --active-key "valid-prefix"; attacker-command; #" --enforce ``` 4. The `exec` tool passes the constructed string to a shell. 5. The shell executes the injected command with the privileges of the OpenClaw process. This attack path requires attacker influence over the `sessionKey` argument. The reviewed func ...[truncated 929 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Do not construct shell command strings from dynamic input.** Invoke the executable through a process API that accepts an argument array and explicitly disables shell interpretation: ```javascript const { spawn } = require('node:child_process'); async function protectSession(sessionKey) { validateSessionKey(sessionKey); return new Promise((resolve, reject) => { const child = spawn( 'openclaw', ['sessions', 'cleanup', '--active-key', sessionKey, '--enforce'], { shell: false } ); let stdout = ''; let stderr = ''; child.stdout.on('data', data => { stdout += data; }); child.stderr.on('data', data => { stderr += data; }); child.on('error', reject); child.on('close', code => { if (code === 0) { resolve({ stdout, stderr, code }); } else { reject(new Error(`openclaw exited with code ${code}: ${stderr}`)); } }); }); } ``` 2. **Validate the session key before execution.** Enforce the documented subagent-session structure and a conservative character allowlist. Reject unexpected whitespace, quotes, shell metacharacters, control characters, and excessive lengths. Validation should reflect the authoritative OpenClaw session-key specification rather than accepting arbitrary strings. 3. **Verify provenance.** Where possible, accept only session keys returned directly by trusted `sessions_list` or `sessions_spawn` operations. Do not use values copied from untrusted messages or external input without independent verification. 4. **Apply least privilege.** Run the OpenClaw process under a dedicated, restricted operating-system account with minimal filesystem and credential access. Do not grant unnecessary administrative privileges. 5. **Correct the documentation.** Replace the unsafe `exec` examples in `SKILL.md:151-153` and `SKILL.md: ...[truncated 331 chars]