T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/selfie.js:69
- Finding
- Shell Command Injection Through Configurable Scene Prompts<![CDATA[ ## Vulnerability Details **File Location**: `scripts/selfie.js`, lines 69-78 **Vulnerability Type**: OS command injection through shell-based process execution **Risk Level**: High ### Vulnerable Code ```javascript // 调用图像生成脚本 const prompt = selectedScene.prompt; const command = `python3 "${IMAGE_GENERATE_SCRIPT}" "${prompt}"`; console.log(`⏳ 生成中...（这可能需要几秒钟）`); execSync(command, { stdio: 'inherit', cwd: path.dirname(IMAGE_GENERATE_SCRIPT) }); ``` ### Technical Analysis The scene prompt is read from `config/scenes.json` and interpolated directly into a command string passed to `child_process.execSync()`. This API executes the supplied string through a system shell. Wrapping `prompt` in double quotes does not make it safe. A prompt containing a double quote can terminate the intended argument and append shell metacharacters or additional commands. The documentation explicitly supports adding custom scenes and prompts, so the configuration is an expected input surface rather than an immutable internal constant. For example, a prompt equivalent to the following could escape the quoted argument: ```text "; touch /tmp/cat-selfie-command-injection; # ``` This would produce a shell command structurally equivalent to: ```bash python3 "/path/to/image_generate.py" ""; touch /tmp/cat-selfie-command-injection; #" ``` The injected command would execute with the same operating-system identity and environment as the Node.js process. ### Attack Path 1. An attacker modifies `config/scenes.json`, distributes a malicious custom scene, or persuades a user to add an attacker-controlled prompt. 2. The malicious prompt includes a quote followed by shell syntax. 3. The user invokes `selfie.js` with the malicious scene ID or allows it to be selected randomly. 4. `generateSelfie()` reads the malicious prompt from the scene configuration. 5. The prompt is interpolated into the `command` string without shell escaping. 6. `execSync()` passes the command to th ...[truncated 666 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Avoid invoking a shell. Pass the executable and its arguments separately with `execFileSync()` or `spawnSync()`: ```javascript const { execFileSync } = require('child_process'); execFileSync( 'python3', [IMAGE_GENERATE_SCRIPT, prompt], { stdio: 'inherit', cwd: path.dirname(IMAGE_GENERATE_SCRIPT) } ); ``` Additional hardening measures should include: 1. Validate that every scene has string-valued `id`, `name`, `emoji`, and `prompt` properties. 2. Impose a reasonable maximum prompt length to prevent resource abuse. 3. Reject control characters if they are not required by the image-generation interface. 4. Keep `shell: false` if the implementation is changed to `spawnSync()`. 5. Restrict write access to the Skill configuration so untrusted local processes cannot silently replace scene prompts. 6. Add a regression test containing quotes and shell metacharacters and verify that they are delivered only as a single Python argument. ]]>