T09 · Insecure Skill Coding Practices
- Location
scripts/verify-browser.sh:59- Finding
User-Controlled Screenshot Path Enables Arbitrary JavaScript Execution
- Content
View full analysis
{ // ... await page.screenshot({ path: '$SCREENSHOT_PATH', fullPage: false }); console.log('截图保存: $SCREENSHOT_PATH'); await browser.close(); })().catch(e => { console.error('❌ 验证失败:', e.message); process.exit(1); }); " ``` ### Technical Analysis The second command-line argument is assigned to `SCREENSHOT_PATH` and interpolated directly into JavaScript source code passed to `node -e`. The value is placed inside a single-quoted JavaScript string without escaping quotes, backslashes, line breaks, or other JavaScript syntax. An attacker who can influence the screenshot path can terminate the string literal and insert additional JavaScript statements. Because the generated program runs in Node.js rather than an isolated browser context, injected code can import modules such as `child_process`, access files, read environment variables, and execute operating-system commands. Shell quoting does not make this safe: the variable is expanded before Node.js parses the resulting source, and the expanded value remains attacker-controlled JavaScript. ### Attack Path 1. An attacker convinces a user or automation system to run `verify-browser.sh` with a crafted second argument. 2. The crafted screenshot path closes the `path` string and object expression. 3. The argument inserts an additional Node.js statement, such as a call through `require('child_process')`. 4. The script builds the inline program through shell interpolation. 5. `node -e` parses and executes the injected statement. 6. The attacker’s command runs with the privileges and environment of the account invoking the Skill. ### Impact A ...[truncated 613 chars]- Remediation
View remediation
{ const browser = await chromium.launch({ headless: true }); const page = await browser.newPage(); await page.screenshot({ path: screenshotPath, fullPage: false }); await browser.close(); })(); NODE ``` Additional hardening should include: 1. Resolve the supplied path with `path.resolve`. 2. Restrict output to an approved directory if arbitrary destinations are unnecessary. 3. Reject null bytes and unsupported file extensions. 4. Avoid all user-controlled interpolation into `node -e` source. 5. Add regression tests using paths containing quotes, backslashes, newlines, and JavaScript syntax. ]]>
