T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/index.js:10
- Finding
- OS Command Injection Through Shell-Based Python Invocation<![CDATA[ ## Vulnerability Details **File Location**: `scripts/index.js`, lines 1 and 10-14; user-controlled data reaches the vulnerable function at line 59 **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```javascript const { exec } = require('child_process'); function executePython(message) { return new Promise((resolve, reject) => { const pythonCmd = `python "${PYTHON_SCRIPT_PATH}" "${message.replace(/"/g, '\\"')}"`; exec(pythonCmd, (error, stdout, stderr) => { if (error) { console.error(`执行错误：${error.message}`); if (stderr) { console.error(`Stderr: ${stderr}`); } ``` The command handler passes agent-controlled command text to this function: ```javascript if (lowerCmd.startsWith('采购')) { try { const result = await executePython(lowerCmd); return { reply: result }; ``` ### Technical Analysis The Skill constructs a command-line string containing the untrusted `message` value and executes it using `child_process.exec()`. Unlike process APIs that accept an executable and argument array, `exec()` invokes a command shell. The attempted escaping operation: ```javascript message.replace(/"/g, '\\"') ``` does not provide reliable shell quoting. In particular, backslash is not a general escape character for quotation marks under Windows `cmd.exe`. An attacker can introduce a quotation mark to terminate the intended argument and then supply shell control operators. The shell interprets these operators before `add_purchase.py` can validate the purchase command. The `startsWith('采购')` check is not a security boundary. An input can begin with the required prefix while still containing shell syntax later in the string. ### Attack Path 1. An attacker submits a command beginning with the accepted `采购` prefix. 2. The complete attacker-controlled string is passed to `executePython()`. 3. A quotation ...[truncated 1376 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Eliminate shell interpretation by replacing `exec()` with `execFile()` or `spawn()` and passing arguments separately: ```javascript const { execFile } = require('child_process'); function executePython(message) { return new Promise((resolve) => { execFile( 'python', [PYTHON_SCRIPT_PATH, message], { shell: false, windowsHide: true, timeout: 10000, maxBuffer: 1024 * 1024 }, (error, stdout, stderr) => { // Handle the result without constructing a shell command. } ); }); } ``` 2. Resolve the Python script relative to the installed Skill directory rather than using a hard-coded, user-specific path: ```javascript const PYTHON_SCRIPT_PATH = path.join(__dirname, 'add_purchase.py'); ``` 3. Validate the command before starting another process: - Enforce a reasonable maximum input length. - Require the documented purchase-command structure. - Reject control characters and unexpected line breaks. - Validate the date, item name, and price as separate fields. 4. Run the Skill under a least-privileged account with access only to the required workbook and Skill files. 5. Do not attempt to repair this issue by adding more shell escaping. Avoiding the shell entirely is the robust mitigation. ]]>