- Location
- scripts/narrate-pipeline.mjs:145
- Finding
- Shell Command Injection Through Narration Output Paths<![CDATA[
## Vulnerability Details
**File Location**: `scripts/narrate-pipeline.mjs:145-162`
**Vulnerability Type**: OS command injection
**Risk Level**: High
### Vulnerable Code
```js
function ffmpegConcat(inputs, output) {
// Use the concat demuxer to merge identically encoded MP3 files
const listFile = output + '.list';
fs.writeFileSync(
listFile,
inputs.map((p) => `file '${p.replace(/'/g, "'\\''")}'`).join('\n'),
);
execSync(
`ffmpeg -y -f concat -safe 0 -i "${listFile}" -c copy "${output}"`,
{ stdio: ['ignore', 'pipe', 'pipe'] },
);
fs.unlinkSync(listFile);
}
function makeSilence(duration, outPath) {
execSync(
`ffmpeg -y -f lavfi -i anullsrc=r=24000:cl=mono -t ${duration} -q:a 9 -acodec libmp3lame "${outPath}"`,
{ stdio: ['ignore', 'pipe', 'pipe'] },
);
}
```
The affected values originate from caller-controlled output arguments:
```js
const outDir = path.resolve(args.outDir);
const audioDir = path.join(outDir, 'audio');
const tmpDir = path.join(outDir, '.tmp');
const gapFile = path.join(tmpDir, 'gap.mp3');
if (gap > 0) makeSilence(gap, gapFile);
const voiceoverPath = path.join(outDir, 'voiceover.mp3');
ffmpegConcat(sceneAudioFiles, voiceoverPath);
```
### Technical Analysis
`execSync()` invokes a shell. The code constructs shell command strings by interpolating filesystem paths directly inside double quotes. Double quotes do not make arbitrary input safe when the input itself can contain a double quote, command substitution, backticks, or other shell metacharacters.
The `--out-dir` argument is resolved as a path but is not restricted to safe characters. Path normalization does not perform shell escaping. A crafted output directory can therefore terminate the quoted argument and append another shell command.
The `gap` value is converted with `parseFloat()`, reducing direct string injection through that field, but it is not checked with `Number.isFinite()` or constrained to a sensible range. The path inte
...[truncated 1214 chars]
- Remediation
- <![CDATA[
## Remediation Suggestions
1. Replace shell-based `execSync()` calls with argument-array execution:
```js
execFileSync('ffmpeg', [
'-y',
'-f', 'concat',
'-safe', '0',
'-i', listFile,
'-c', 'copy',
output,
], {
stdio: ['ignore', 'pipe', 'pipe'],
});
```
```js
execFileSync('ffmpeg', [
'-y',
'-f', 'lavfi',
'-i', 'anullsrc=r=24000:cl=mono',
'-t', String(duration),
'-q:a', '9',
'-acodec', 'libmp3lame',
outPath,
], {
stdio: ['ignore', 'pipe', 'pipe'],
});
```
2. Validate `gap` using `Number.isFinite(gap)` and enforce a reasonable minimum and maximum.
3. Reject NUL bytes and unexpected control characters in paths.
4. Consider constraining output to a caller-approved project directory.
5. Add regression tests using filenames containing quotes, spaces, dollar signs, backticks, semicolons, and newlines.
6. Ensure temporary list files are removed in a `finally` block even when FFmpeg fails.
]]>