T09 · Insecure Skill Coding Practices
- Location
scripts/screenshot.sh:62- Finding
Arbitrary Node.js Code Execution Through Unescaped Script Arguments
- Content
View full analysis
document.documentElement.scrollHeight); await page.setViewport({width: $WIDTH, height: height}); await new Promise(r => setTimeout(r, 500)); await page.screenshot({path: '$OUTPUT', fullPage: true}); ``` ### Technical Analysis The script constructs a JavaScript program for `node -e` by directly interpolating shell variables into its source code. The attacker-controlled `WIDTH`, `NAME`, and `OUTPUT` values are neither validated nor encoded as JavaScript values. `WIDTH` is inserted directly into an object literal. An attacker can provide JavaScript syntax instead of an integer. `NAME` and `OUTPUT` are inserted into single-quoted JavaScript strings, so a filename or output path containing a single quote and additional JavaScript can terminate the string and inject new statements. Shell quoting does not prevent this vulnerability because expansion occurs inside the double-quoted argument passed to `node -e`. The resulting expanded text is subsequently parsed and executed as JavaScript by Node.js. ### Attack Path 1. An attacker controls the width argument, output path, or name of the input HTML file. 2. The attacker supplies a value containing syntactically valid JavaScript, such as a width expression that invokes Node.js functionality. 3. The shell interpolates the value into the JavaScript source passed to `node -e`. 4. Node.js parses the injected content as part of the program. 5. The injected code executes with the privileges and environment of the user running the skill. ### Impact Assessment Successful exploitation provides arbitrary Node.js code exe ...[truncated 432 chars]- Remediation
View remediation
&2 exit 1 ;; esac if [ "$WIDTH" -lt 1 ] || [ "$WIDTH" -gt 10000 ]; then echo "ERROR: width is outside the permitted range" >&2 exit 1 fi NODE_PATH="$NODE_PATH" node /path/to/screenshot.js \ "$CHROME" "$WIDTH" "$PORT" "$NAME" "$OUTPUT" ``` The separate JavaScript file should consume these values exclusively through `process.argv` and should never evaluate them as source code. ]]>
