T09 · Insecure Skill Coding Practices
Error
- Location
- index.js:47
- Finding
- OS Command Injection Through User-Controlled CLI Inputs<![CDATA[ ## Vulnerability Details **File Location**: `index.js`, lines 47–57 **Vulnerability Type**: OS command injection through unsafe shell-command construction **Risk Level**: High ### Vulnerable Code ```js // 構建 curl 命令 const curlCmd = `curl -s -X POST ${options.url}/chat_direct \\\n` + ` -H "Content-Type: application/json" \\\n` + ` -d '{"question":${JSON.stringify(query)},"session_id":"${sessionId}","lang":"${options.lang || 'zh'}"}'`; console.log(`📡 發送查詢到 ${options.url}/chat_direct`); console.log(`🆔 Session ID: ${sessionId}`); // 執行 curl const { stdout } = await execPromise(curlCmd); ``` ### Technical Analysis The implementation constructs a shell command by interpolating user-controlled values and passes the resulting string to `child_process.exec` through `execPromise()`. The following values are attacker-controllable: - `options.url`, supplied through `--url` - `query`, supplied as the positional query argument or read from a batch file - `options.lang`, supplied through `--lang` `options.url` is inserted without quoting or validation, allowing shell metacharacters such as semicolons, command substitutions, redirections, and pipes to change the command executed by the shell. Although `query` is passed through `JSON.stringify()`, that function only produces JSON-safe syntax. It does not provide shell escaping. The generated JSON is enclosed in a shell single-quoted argument, but an apostrophe contained in a query remains present after `JSON.stringify()` and can terminate that shell quote. The remainder can then be interpreted as shell commands. The `lang` value is also interpolated without validation against the documented `zh` and `en` choices. It is inside the same shell-quoted JSON argument and can similarly participate in quote termination. Using `exec()` is the underlying dangerous sink because it invokes a command shell rather than passing arguments directly to `curl`. ### Attack Path #### Injecti ...[truncated 2232 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Do not invoke a command shell.** Replace `child_process.exec()` with Node.js `fetch()` or `http.request()` so no shell command is constructed. ```js async function callApi(query, options, sessionId) { const baseUrl = validateServiceUrl(options.url); const endpoint = new URL('/chat_direct', baseUrl); const response = await fetch(endpoint, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ question: query, session_id: sessionId, lang: options.lang }), signal: AbortSignal.timeout(30_000) }); if (!response.ok) { throw new Error(`API returned HTTP ${response.status}`); } return response.json(); } ``` 2. **If `curl` must be retained, use `execFile()` or `spawn()` with an argument array.** Do not concatenate values into a command string. ```js const payload = JSON.stringify({ question: query, session_id: sessionId, lang: options.lang }); const { stdout } = await execFilePromise('curl', [ '-sS', '-X', 'POST', `${validatedUrl}/chat_direct`, '-H', 'Content-Type: application/json', '--data-binary', payload ]); ``` 3. **Validate the language option explicitly.** ```js if (!['zh', 'en'].includes(options.lang)) { throw new Error('Invalid language; expected zh or en'); } ``` 4. **Parse and validate the destination URL.** The documentation describes the endpoint as local-only, but the implementation accepts unrestricted destinations. Use `new URL()` and permit only the intended protocol, port, and local/private hosts. Reject embedded credentials, unexpected schemes, malformed ports, and public destinations if they are outside the skill's intended behavior. 5. **Apply an effective timeout.** The declared `--timeout` option is currently unused. Enforce a bounded request timeout to prevent indefinite hangs. 6. **Add regression tests.** Include queries, language values, and URLs containing apostrophes, semicolo ...[truncated 224 chars]