T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/navigate-pmos.js:28
- Finding
- Shell Command Injection Through User-Supplied Element References## Vulnerability Details **File Location**: `scripts/navigate-pmos.js:28-39`, `scripts/navigate-pmos.js:58-64`, and `scripts/navigate-pmos.js:151-158` **Vulnerability Type**: OS command injection **Risk Level**: High The Node.js navigation script accepts an element reference from standard input and incorporates it directly into a shell command executed by `execSync()`. **Vulnerable code:** ```js // Execute an OpenClaw command function runCommand(cmd, silent = false) { try { const output = execSync(cmd, { encoding: 'utf-8', stdio: silent ? 'pipe' : 'inherit' }); return output; } catch (error) { if (!silent) { console.error(`Command execution failed: ${cmd}`); console.error(error.message); } throw error; } } // Click a menu item function clickMenuItem(ref, targetId) { console.log(`Clicking menu item (ref: ${ref})...`); const cmd = targetId ? `openclaw browser act click --ref ${ref} --targetId ${targetId}` : `openclaw browser act click --ref ${ref}`; runCommand(cmd); } const ref = await new Promise(resolve => { rl.question('Enter the element reference, for example e78, or leave blank to skip: ', resolve); }); if (ref) { clickMenuItem(ref, currentTabId); } ``` ### Technical Analysis `child_process.execSync()` executes a string through the operating-system shell. The `ref` value is obtained interactively and is concatenated into that command without syntax validation, escaping, or separation into an argument array. Consequently, shell metacharacters contained in `ref` are interpreted by the shell rather than treated as part of an OpenClaw argument. An input conceptually shaped like `e78; attacker-command` can terminate or extend the intended command and invoke an additional local command. The `targetId` value is also interpolated into command strings without validation. It originates from the output of ...[truncated 1469 chars]
- Remediation
- ## Remediation Suggestions 1. Replace `execSync()` with `execFileSync()` or `spawnSync()` and pass each command-line argument as a separate array element. Do not enable shell execution. 2. Validate element references using an allowlist matching the documented format. For example, permit only the letter `e` followed by one or more decimal digits. 3. Validate tab identifiers against the exact format documented by OpenClaw before using them. 4. Reject invalid input explicitly rather than passing it to the command runner. 5. Keep static values such as `openclaw`, `browser`, `act`, and `click` separate from runtime arguments. 6. Add automated tests covering semicolons, command substitutions, pipes, redirection characters, quotes, whitespace, and newline injection. 7. Run the Skill with the least-privileged operating-system account necessary for browser navigation. A safer implementation should follow this structure: ```js const { execFileSync } = require('child_process'); function validateRef(ref) { if (!/^e\d+$/.test(ref)) { throw new Error('Invalid accessibility element reference'); } } function clickMenuItem(ref, targetId) { validateRef(ref); const args = ['browser', 'act', 'click', '--ref', ref]; if (targetId) { validateTargetId(targetId); args.push('--targetId', targetId); } execFileSync('openclaw', args, { encoding: 'utf-8', stdio: 'inherit' }); } ```