T09 · Insecure Skill Coding Practices
Error
- Location
index.js:20- Finding
Shell Command Injection in Exported Image Wrapper Functions
- Content
View full analysis
{ let cmd = `node "${TOOL_PATH}" send --file "${filePath}"`; if (target) cmd += ` --target "${target}"`; if (message) cmd += ` --message "${message}"`; exec(cmd, (error, stdout, stderr) => { if (error) { reject(new Error(stderr || error.message)); return; } try { const result = JSON.parse(stdout); resolve(result); } catch (e) { resolve({ success: true, output: stdout }); } }); }); } ``` ```javascript function uploadImage(filePath) { return new Promise((resolve, reject) => { const cmd = `node "${TOOL_PATH}" upload --file "${filePath}"`; exec(cmd, (error, stdout, stderr) => { if (error) { reject(new Error(stderr || error.message)); return; } try { const result = JSON.parse(stdout); resolve(result); } catch (e) { resolve({ success: true, output: stdout }); } }); }); } ``` ### Technical Analysis The exported `sendImage()` and `uploadImage()` functions interpolate caller-controlled values into command strings passed to `child_process.exec()`. The affected inputs are `target`, `filePath`, and `message`. `exec()` invokes a system shell. Placing values inside double quotes does not make them safe because shell command substitution can still occur inside double-quoted strings, and an embedded quote can terminate the intended argument. Consequently, an attacker can provide shell metacharacters or command-substitution syntax that the shell interprets as executable commands. The functions are exported through `module.exports`, so this vulnerability is reachable not ...[truncated 1542 chars]- Remediation
View remediation
{ const args = [TOOL_PATH, 'send', '--file', filePath]; if (target) { args.push('--target', target); } if (message) { args.push('--message', message); } execFile(process.execPath, args, { shell: false }, (error, stdout, stderr) => { if (error) { reject(new Error(stderr || error.message)); return; } try { resolve(JSON.parse(stdout)); } catch { resolve({ success: true, output: stdout }); } }); }); } ``` Apply the same pattern to `uploadImage()`. Additional hardening should include: - Validate Feishu target identifiers against their expected syntax. - Resolve and normalize file paths before use. - Restrict readable image files to explicitly authorized directories when inputs cross a trust boundary. - Reject null bytes and unexpected control characters. - Set execution timeouts and output-size limits. - Avoid attempting to make shell commands safe through manual quoting; eliminate the shell entirely. ]]>