T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/video-editor.js:22
- Finding
- Arbitrary Command Execution Through Shell-Based FFmpeg and FFprobe Invocation<![CDATA[ ## Vulnerability Details **File Location**: `scripts/video-editor.js:22-29` and `scripts/video-editor.js:233-241` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```js async getVideoInfo(videoPath) { const cmd = `"${this.config.ffprobePath}" -v quiet -print_format json -show_format -show_streams "${videoPath}"`; try { const output = execSync(cmd, { encoding: 'utf8' }); const info = JSON.parse(output); ``` ```js executeFfmpeg(cmd) { return new Promise((resolve, reject) => { console.log(`执行: ${cmd}`); const process = spawn(cmd, [], { shell: true, stdio: ['ignore', 'pipe', 'pipe'] }); ``` Additional command construction involving configurable input appears in the watermark operation at `scripts/video-editor.js:99-123`: ```js async addWatermark(input, output, options = {}) { const { text = 'AutoClip Pro', position = 'bottom-right', fontSize = 20, fontColor = 'white', opacity = 0.5 } = options; const positions = { 'top-left': 'x=10:y=10', 'top-right': 'x=w-tw-10:y=10', 'bottom-left': 'x=10:y=h-th-10', 'bottom-right': 'x=w-tw-10:y=h-th-10', 'center': 'x=(w-tw)/2:y=(h-th)/2' }; const pos = positions[position] || positions['bottom-right']; const filter = `drawtext=text='${text}':fontsize=${fontSize}:fontcolor=${fontColor}@${opacity}:${pos}`; const cmd = `"${this.config.ffmpegPath}" -y -i "${input}" -vf "${filter}" -c:v libx264 -preset medium -crf 23 -c:a copy "${output}"`; return this.executeFfmpeg(cmd); } ``` ### Technical Analysis The implementation constructs complete command lines through string interpolation and then executes them through a system shell. Quoting a value with double or single quotation marks does not make it safe when the value itself can contain quotation marks or shell metacharacters. The following values can enter shell command strings without robust validation or argument separation: - ...[truncated 2167 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace command-string execution with argument-array execution: - Use `execFile()` or `spawn()` with `shell: false`. - Pass the executable separately from its arguments. - Never concatenate input into a complete shell command. 2. Refactor FFprobe invocation as follows: ```js const { execFile } = require('child_process'); const { promisify } = require('util'); const execFileAsync = promisify(execFile); const { stdout } = await execFileAsync( this.config.ffprobePath, [ '-v', 'quiet', '-print_format', 'json', '-show_format', '-show_streams', videoPath ], { encoding: 'utf8' } ); ``` 3. Refactor FFmpeg execution to accept an argument array: ```js executeFfmpeg(args) { return new Promise((resolve, reject) => { const child = spawn(this.config.ffmpegPath, args, { shell: false, stdio: ['ignore', 'pipe', 'pipe'] }); // Handle output and termination here. }); } ``` 4. Apply strict allowlists and type validation: - Restrict resolution and transition types to known values. - Require durations, widths, opacity, volume, and font sizes to be finite numbers in safe ranges. - Resolve and validate subtitle and media paths before use. - Reject control characters and unexpected values in executable paths. 5. Escape user-provided content according to FFmpeg filter syntax. Shell safety and FFmpeg filter escaping are separate requirements; argument arrays eliminate shell injection but do not prevent malformed or injected FFmpeg filter expressions. 6. Add automated tests using filenames and text containing quotes, semicolons, dollar signs, backticks, spaces, and newlines. Verify that these values are passed as literal arguments and never interpreted by a shell. ]]>