T09 · Insecure Skill Coding Practices
Error
- Location
- index.js:17
- Finding
- Shell Command Injection Through Repository Paths and Date Arguments<![CDATA[ ## Vulnerability Details **File Location**: `index.js`, lines 13–27; attacker-controlled values enter through lines 123–128 and 144–155 **Vulnerability Type**: OS command injection caused by unsafe shell command construction **Risk Level**: High ### Complete Vulnerable Code Snippet ```js function getGitLogs(since, until, repos = ['.']) { const commits = []; for (const repo of repos) { try { const cmd = `cd "${repo}" && git log --since="${since}" --until="${until}" --pretty=format:"%h|%s|%an|%ad" --date=short 2>/dev/null || echo ""`; const output = execSync(cmd, { encoding: 'utf-8' }).trim(); if (output) { output.split('\n').forEach(line => { const [hash, message, author, date] = line.split('|'); if (hash && message) { commits.push({ hash, message, author, date, repo }); } }); } } catch (e) { // 忽略错误 } } } ``` The command-line date values are accepted without validation: ```js case 'range': const fromIdx = args.indexOf('--from'); const toIdx = args.indexOf('--to'); since = fromIdx !== -1 ? args[fromIdx + 1] : today.toISOString().split('T')[0]; until = toIdx !== -1 ? args[toIdx + 1] : new Date(today.getTime() + 86400000).toISOString().split('T')[0]; break; ``` Repository paths are loaded from a working-directory configuration file and passed to the vulnerable function: ```js let config = { git: { repos: ['.'] }, output: { language: 'zh-CN' } }; const configPath = path.join(process.cwd(), '.reportrc.json'); if (fs.existsSync(configPath)) { try { config = { ...config, ...JSON.parse(fs.readFileSync(configPath, 'utf-8')) }; } catch (e) {} } // 获取提交并生成报告 const commits = getGitLogs(since, until, config.git.repos); ``` ### Technical Analysis The application constructs a shell command by directly interpolating `repo`, `since`, and `until`, then executes it through `execSync()`. These values originate from `.reportrc.json` ...[truncated 2261 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Remove the shell from the execution path.** Use `execFileSync()` or `spawnSync()` with a separate argument array and the `cwd` option: ```js const { execFileSync } = require('child_process'); function getGitLogs(since, until, repos = ['.']) { const commits = []; for (const repo of repos) { const resolvedRepo = path.resolve(repo); try { const output = execFileSync( 'git', [ 'log', `--since=${since}`, `--until=${until}`, '--pretty=format:%h|%s|%an|%ad', '--date=short' ], { cwd: resolvedRepo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] } ).trim(); // Parse output as before. } catch (error) { // Report or safely handle the error. } } return commits; } ``` 2. **Strictly validate date arguments.** Require an exact `YYYY-MM-DD` representation and verify that it denotes a real date. Reject missing values after `--from` or `--to` rather than silently accepting `undefined`. 3. **Validate configuration structure.** Confirm that `config.git` is an object, `config.git.repos` is an array, and each repository entry is a non-empty string. 4. **Constrain repository paths.** Resolve and canonicalize every path. If the application should only inspect approved directories, verify that each canonical path remains under an explicitly allowed root and is an actual Git working tree. 5. **Avoid silent exception handling.** Emit a safe diagnostic that identifies which repository failed without exposing sensitive details. Silent failures can conceal attempted exploitation and operational errors. 6. **Add regression tests.** Test repository and date values containing command substitution, shell metacharacters, whitespace, quotes, and option-like prefixes. Verify that no external command or marker file is created. ]]>