T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:25
- Finding
- User-Controlled Water Quantity Injected into Executable JavaScript## Vulnerability Details **File Location**: `SKILL.md`, line 25 **Vulnerability Type**: JavaScript code injection through unsafe source-code substitution **Risk Level**: High ### Vulnerable Code ```bash node -e "const fs=require('fs');const f='{baseDir}/health-data.json';let d={water:[],sleep:[]};try{d=JSON.parse(fs.readFileSync(f))}catch(e){}d.water.push({time:new Date().toISOString(),cups:CUPS});fs.writeFileSync(f,JSON.stringify(d));console.log('Da ghi: '+CUPS+' coc')" ``` The accompanying instruction directs the agent to replace `CUPS` with a number obtained from user input. It does not require strict numeric parsing, range validation, or safe argument passing. ### Technical Analysis `CUPS` is embedded directly into JavaScript source passed to `node -e`. If an agent substitutes the raw or insufficiently validated user-controlled value, an attacker can terminate the intended expression and append arbitrary JavaScript. For example, a value shaped like the following can escape the intended `cups` property expression: ```javascript 0});require('child_process').execSync('id');/* ``` After substitution, Node.js evaluates both the intended record operation and the injected statement. The trailing comment can suppress the remaining generated source. Because `require` is available, injected code can access Node.js filesystem, process, networking, and child-process capabilities. This issue does not require a third-party dependency or a remote payload. It results from constructing executable source code with a user-derived value. ### Attack Path 1. An attacker submits a water-tracking request containing a crafted value instead of a normal cup count. 2. The agent follows the skill instruction and substitutes that value for `CUPS` without strict numeric validation. 3. The resulting command is supplied to `node -e`. 4. Node.js parses the crafted value as executable source rather than merely as health-record data. 5. Th ...[truncated 600 chars]
- Remediation
- ## Remediation Suggestions Do not insert user-controlled values into JavaScript source. Keep the script fixed and pass the quantity as a separate argument or through a structured input channel. Parse and validate it before use. A hardened implementation should: 1. Pass the value as a command-line argument rather than interpolating it into the `node -e` program. 2. Convert it with `Number(...)`. 3. Reject values for which `Number.isFinite(...)` is false. 4. Enforce an appropriate range and, if required by the data model, integer-only input. 5. Avoid invoking a shell where a direct Node.js process call or standalone script is available. 6. Report invalid input without modifying the JSON file. Example validation logic: ```javascript const cups = Number(process.argv[1]); if (!Number.isFinite(cups) || !Number.isInteger(cups) || cups < 0 || cups > 100) { throw new Error('Invalid cup count'); } ``` Prefer placing this fixed logic in a reviewed script and invoking it with an argument array so input is treated exclusively as data.