T09 · Insecure Skill Coding Practices
- Location
scripts/navigate-pmos.js:28- Finding
Shell Command Injection Through User-Supplied Element References
- Content
View full analysis
Vulnerability Details
File Location:
scripts/navigate-pmos.js:28-39,scripts/navigate-pmos.js:58-64, andscripts/navigate-pmos.js:151-158
Vulnerability Type: OS command injection
Risk Level: HighThe 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. Therefvalue is obtained interactively and is concatenated into that command without syntax validation, escaping, or separation into an argument array.Consequently, shell metacharacters contained in
refare interpreted by the shell rather than treated as part of an OpenClaw argument. An input conceptually shaped likee78; attacker-commandcan terminate or extend the intended command and invoke an additional local command.The
targetIdvalue is also interpolated into command strings without validation. It originates from the output of ...[truncated 1469 chars]- Remediation
View remediation
Remediation Suggestions
- Replace
execSync()withexecFileSync()orspawnSync()and pass each command-line argument as a separate array element. Do not enable shell execution. - Validate element references using an allowlist matching the documented format. For example, permit only the letter
efollowed by one or more decimal digits. - Validate tab identifiers against the exact format documented by OpenClaw before using them.
- Reject invalid input explicitly rather than passing it to the command runner.
- Keep static values such as
openclaw,browser,act, andclickseparate from runtime arguments. - Add automated tests covering semicolons, command substitutions, pipes, redirection characters, quotes, whitespace, and newline injection.
- 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' }); }- Replace
