T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/create-employee.sh:187
- Finding
- Arbitrary JavaScript Execution Through Unsafely Constructed node -e Program<![CDATA[ ## Vulnerability Details **File Location**: `scripts/create-employee.sh:187-203` **Vulnerability Type**: Shell-to-JavaScript code injection **Risk Level**: High ### Vulnerable Code ```bash if command -v node &> /dev/null; then node -e " const fs = require('fs'); const data = JSON.parse(fs.readFileSync('$EMPLOYEE_JSON', 'utf-8')); const exists = data.employees.find(e => e.id === '$ROLE'); if (!exists) { data.employees.push({ id: '$ROLE', name: '$AGENT_NAME', phone: '$PHONE', chatId: '$CHAT_ID', sessionId: '$SESSION_ID', workspace: '$WORKSPACE_PATH', agentDir: '$AGENT_DIR', status: 'pending_onboarding', createdAt: new Date().toISOString() }); } fs.writeFileSync('$EMPLOYEE_JSON', JSON.stringify(data, null, 2)); console.log(' ✅ 员工信息已添加'); " 2>/dev/null || echo " ⚠️ 无法更新" fi ``` ### Technical Analysis The script constructs an executable JavaScript program by directly interpolating shell variables into a `node -e` argument. Several interpolated values are derived from command-line input, including `ROLE`, `AGENT_NAME`, and `PHONE`. Other values originate from filesystem paths or the Feishu API response. Shell quoting does not make these values safe inside JavaScript string literals. An input containing a single quote and valid JavaScript syntax can terminate or restructure the intended string literal. For example, an attacker can supply a value that adds another object property whose value executes an expression such as `require('child_process').execSync(...)`. This is not merely malformed JSON injection: the affected text is evaluated as JavaScript source by Node.js. ### Attack Path 1. The attacker obtains the ability to invoke `create-employee.sh`, either directly or indirectly through `batch-create.sh` and an attacker-controlled CSV file. 2. The attacker places a JavaScript expression payload in an employee field such as the phone number or employee name. 3. The script performs its earlier creation ...[truncated 999 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Never interpolate shell variables into JavaScript source. - Place values in environment variables or pass them as separate positional arguments: ```bash EMPLOYEE_JSON="$EMPLOYEE_JSON" \ ROLE="$ROLE" \ AGENT_NAME="$AGENT_NAME" \ PHONE="$PHONE" \ node <<'NODE' const fs = require('fs'); const path = process.env.EMPLOYEE_JSON; const data = JSON.parse(fs.readFileSync(path, 'utf8')); if (!data.employees.some(employee => employee.id === process.env.ROLE)) { data.employees.push({ id: process.env.ROLE, name: process.env.AGENT_NAME, phone: process.env.PHONE }); } fs.writeFileSync(path, JSON.stringify(data, null, 2), { mode: 0o600 }); NODE ``` - Pass every remaining field through `process.env` or a structured input file rather than source interpolation. - Validate role, phone, chat ID, and session ID formats before use. - Use a JSON-aware implementation for all configuration updates. - Add regression tests containing quotes, backslashes, newlines, JavaScript syntax, and shell metacharacters. - Restrict employee-record permissions because they contain personal data. ]]>