T09 · Insecure Skill Coding Practices
- Location
scripts/generate_svg.js:140- Finding
OS Command Injection Through Attacker-Controlled Output Path
- Content
View full analysis
Vulnerability Details
File Location:
scripts/generate_svg.js:140-147
Input Sources:scripts/generate_svg.js:21-31andscripts/generate_svg.js:113-116
Vulnerability Type: OS command injection
Risk Level: HighThe output path is populated from command-line arguments without validation:
js for (let i = 0; i < args.length; i++) { if (args[i] === '--template' || args[i] === '-t') { template = args[++i]; } else if (args[i] === '--output' || args[i] === '-o') { outputPath = args[++i]; } else if (!args[i].startsWith('-')) { if (!description) { description = args[i]; } else { outputPath = args[i]; } } }Template mode may also select the final command-line argument directly:
js if (template && templates[template]) { const svg = templates[template](400); // Use outputPath from args if provided const finalOutput = args[args.length - 1].endsWith('.png') ? args[args.length - 1] : outputPath; saveAndConvert(svg, finalOutput); console.log(`Template '${template}' saved to ${finalOutput}`); process.exit(0); }The vulnerable command execution segment is:
js function saveAndConvert(svg, output) { const svgPath = output.replace('.png', '.svg'); fs.writeFileSync(svgPath, svg); try { execSync(`rsvg-convert "${svgPath}" -o "${output}"`, { stdio: 'pipe' }); } catch (e) { try { execSync(`convert "${svgPath}" "${output}"`, { stdio: 'pipe' });Technical Analysis
Node.js
execSyncexecutes the supplied string through a system shell. BothsvgPathandoutputare derived from an untrusted command-line argument and are interpolated directly into shell command strings.Wrapping these values in double quotes does not prevent command injection. Shell constructs such as command substitution using
$()or backticks remain active inside double-quoted strings. An ...[truncated 2153 chars]- Remediation
View remediation
Remediation Suggestions
-
Replace shell-based
execSynccalls withexecFileSyncorspawnSync, passing each argument separately:js const path = require('path'); const { execFileSync } = require('child_process'); function saveAndConvert(svg, requestedOutput) { const outputDir = '/tmp/svg-artist'; fs.mkdirSync(outputDir, { recursive: true, mode: 0o700 }); const baseName = path.basename(requestedOutput); if (!/^[A-Za-z0-9._-]+\.png$/i.test(baseName)) { throw new Error('Invalid output filename'); } const output = path.join(outputDir, baseName); const svgPath = path.join( outputDir, `${path.basename(baseName, path.extname(baseName))}.svg` ); fs.writeFileSync(svgPath, svg, { encoding: 'utf8', mode: 0o600, flag: 'wx' }); try { execFileSync('rsvg-convert', [svgPath, '-o', output], { stdio: 'pipe', shell: false }); } catch (error) { execFileSync('convert', [svgPath, output], { stdio: 'pipe', shell: false }); } } -
Resolve output files beneath a dedicated, application-controlled directory. Reject absolute paths, traversal sequences, path separators, and any resolved path outside that directory.
-
Require an explicit
.pngextension rather than usingString.replace('.png', '.svg'). Construct the SVG filename usingpath.parseorpath.basename. -
Create temporary files with restrictive permissions and collision-resistant names. Use exclusive creation where appropriate to reduce overwrite and symlink risks.
-
Run conversion utilities with the least privileges required and a minimal environment. Apply execution timeouts and resource limits to reduce the impact of malformed inputs.
-
Add automated security tests covering
$(), backticks, embedded quotes, semicolons, newlines, abs ...[truncated 42 chars]
-