T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/tts.mjs:76
- Finding
- Unrestricted Output Path Permits Arbitrary File Overwrite<![CDATA[ ## Vulnerability Details **File Location**: `scripts/tts.mjs`, lines 76–105 **Vulnerability Type**: Unrestricted filesystem path and unsafe file overwrite **Risk Level**: Medium ### Vulnerable Code ```js // Prepare output path const outDir = join(process.env.HOME, ".openclaw", "media", "outbound"); mkdirSync(outDir, { recursive: true }); const basePath = outputArg || join(outDir, `tts-${Date.now()}`); const aiffPath = `${basePath}.aiff`; // Generate speech const sayArgs = []; if (voice) sayArgs.push("-v", voice); sayArgs.push("-o", aiffPath, text); try { execFileSync("say", sayArgs, { stdio: ["pipe", "pipe", "pipe"] }); } catch (e) { die(`say failed: ${((e.stderr || "") + (e.stdout || "")).trim()}`, 6); } if (!existsSync(aiffPath) || statSync(aiffPath).size === 0) { die("say produced empty output", 6); } // Convert to ogg/opus if ffmpeg available if (which("ffmpeg")) { const oggPath = `${basePath}.ogg`; try { execFileSync( "ffmpeg", ["-hide_banner", "-loglevel", "error", "-y", "-i", aiffPath, "-c:a", "libopus", "-b:a", "48k", "-vbr", "on", "-compression_level", "10", oggPath], { stdio: ["pipe", "pipe", "pipe"] }, ); ``` ### Technical Analysis The optional `outputArg` is accepted directly as the output base path. The code does not canonicalize the path, restrict it to the intended `~/.openclaw/media/outbound` directory, reject traversal or absolute paths, check for symbolic links, or require creation of a new file. The program appends `.aiff` and `.ogg` to the attacker-influenced base path and passes those paths to native utilities. In particular, `ffmpeg` is invoked with `-y`, which explicitly authorizes overwriting an existing output file without prompting. The `say` utility may likewise replace an existing AIFF output at the selected path. Using `execFileSync` with argument arrays prevents shell-command injection, but it does not prevent filesystem path manipulation or overwrite attacks. ### Attack Pa ...[truncated 1407 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Resolve all output paths against the intended outbound directory and reject paths that escape it: ```js import { resolve, relative, isAbsolute } from "node:path"; const allowedDir = resolve(process.env.HOME, ".openclaw", "media", "outbound"); const requestedBase = outputArg ? resolve(allowedDir, outputArg) : resolve(allowedDir, `tts-${Date.now()}`); const rel = relative(allowedDir, requestedBase); if (rel.startsWith("..") || isAbsolute(rel)) { die("output path must remain inside the outbound media directory", 2); } ``` 2. Prefer accepting only a filename rather than an arbitrary path. Reject path separators, `.`/`..` components, and unexpected filename characters. 3. Do not silently overwrite existing files. Remove `ffmpeg`'s `-y` option or replace it with `-n` so conversion fails if the target already exists. 4. Generate unpredictable, application-controlled filenames using `mkdtemp`, `randomUUID`, or an equivalent secure mechanism rather than relying only on a timestamp. 5. Before writing, use `lstatSync` to reject existing symbolic links and non-regular files. Where possible, create files atomically with exclusive-create semantics. 6. Keep intermediate and final files in a newly created private directory with restrictive permissions, then return the generated path to the caller. 7. If external output locations are a required feature, require explicit trusted-user authorization and perform canonical-path, ownership, type, and overwrite checks before invoking `say` or `ffmpeg`. ]]>