T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/render.mjs:159
- Finding
- Command Injection Through the Audio File Path## Vulnerability Details **File Location**: `scripts/render.mjs:159-162` **Vulnerability Type**: OS command injection **Risk Level**: High ### Vulnerable Code ```js function getAudioDuration(filePath) { try { const result = execSync( `ffprobe -v error -show_entries format=duration -of csv=p=0 "${filePath}"`, {encoding: 'utf-8'} ).trim(); return parseFloat(result); } catch { return null; } } ``` ### Technical Analysis The `filePath` value is derived from the user-controlled `--audio` argument and inserted directly into a shell command passed to `execSync()`. Although the value is enclosed in double quotes, embedded quotes, command substitutions, and shell metacharacters are not escaped. Node.js executes the string through a command shell. Consequently, a malicious audio filename can terminate the quoted argument and append another shell command. The wrapper verifies that the supplied audio file exists, but this does not prevent exploitation because filenames on supported operating systems can contain quotes and shell metacharacters. ### Attack Path 1. An attacker creates or supplies an accessible audio file whose path contains shell syntax. 2. The attacker passes that path through the documented `--audio` option. 3. `resolveFilePath()` resolves the path without removing shell metacharacters. 4. The audio is copied into `public/`, preserving its basename. 5. `getAudioDuration()` embeds the resulting path in the `ffprobe` command string. 6. `execSync()` invokes a shell, which interprets the injected syntax and executes the appended command. A conceptual malicious filename could contain syntax equivalent to: ```text song.mp3"; attacker-command; # ``` ### Impact Assessment Successful exploitation provides arbitrary command execution with the privileges of the user or service running the Skill. The attacker could read or modify files accessible to that accoun ...[truncated 293 chars]
- Remediation
- ## Remediation Suggestions Replace shell-string execution with an API that passes each argument directly to the executable: ```js import {execFileSync} from 'child_process'; function getAudioDuration(filePath) { try { const result = execFileSync( 'ffprobe', [ '-v', 'error', '-show_entries', 'format=duration', '-of', 'csv=p=0', filePath, ], { encoding: 'utf-8', shell: false, } ).trim(); const duration = Number.parseFloat(result); return Number.isFinite(duration) && duration > 0 ? duration : null; } catch { return null; } } ``` Additional hardening should include: - Resolve `ffprobe` to a trusted executable or use a controlled executable search path. - Verify that the input is a regular file before processing it. - Apply file size and media-duration limits to reduce denial-of-service exposure. - Do not attempt to make shell interpolation safe through manual escaping; avoid invoking a shell entirely.