T09 · Insecure Skill Coding Practices
Error
- Location
handler.ts:12- Finding
Shell Command Injection Through an Untrusted Session Key
- Content
View full analysis
{ const cmd = `python3 ${script} ${args.join(" ")}`; console.log(`[secretary-memory] Running: ${cmd}`); try { const { stdout, stderr } = await execAsync(cmd, { timeout: 60000 }); if (stdout) console.log(`[secretary-memory] stdout: ${stdout}`); if (stderr) console.error(`[secretary-memory] stderr: ${stderr}`); } catch (err: any) { console.error(`[secretary-memory] Error: ${err.message}`); } } ``` The affected call sites obtain an argument directly from the event: ```ts const sessionKey = event.sessionKey || "unknown"; await runPython(`${SKILL_SCRIPTS}/session_summary.py`, [ "--session-id", sessionKey, "--verbose" ]); ``` ```ts const sessionKey = event.sessionKey || "unknown"; await runPython(`${SKILL_SCRIPTS}/auto_loader.py`, [ "--session-id", sessionKey ]); ``` ### Technical Analysis `runPython` constructs a shell command by joining argument strings without quoting or escaping them. It then passes that command to `child_process.exec`, which invokes a shell. If an attacker can influence `event.sessionKey`, shell metacharacters in the value will be interpreted as command syntax rather than as part of a Python argument. Relevant metacharacters include command separators, command substitutions, pipes, and output redirections. For example, a session key structurally equivalent to: ```text legitimate-id; attacker-command ``` would produce a command structurally equivalent to: ```sh python3 /root/.openclaw/workspace/skills/secretary-memory/scripts/session_summary.py --session-id legitimate-id; attacker-command --verbose ``` The shell can consequently execute the injected command independently of the intended ...[truncated 1359 chars]- Remediation
View remediation
{ const { stdout, stderr } = await execFileAsync( "python3", [script, ...args], { timeout: 60000 } ); if (stdout) console.log(`[secretary-memory] stdout: ${stdout}`); if (stderr) console.error(`[secretary-memory] stderr: ${stderr}`); } ``` Apply defense in depth by validating `sessionKey` before use. If session identifiers have a defined format, enforce a restrictive allowlist such as letters, digits, underscores, and hyphens, together with a reasonable maximum length. Additional hardening should include: - Run the hook as a dedicated, unprivileged operating-system account. - Avoid logging complete commands containing untrusted or sensitive values. - Resolve and validate the Python script path against an explicit allowlist. - Set output-buffer and timeout limits appropriate to the expected scripts. ]]>