fix: escape shell metacharacters in whisper command (command injection) - #64
Open
zachariah-mithani wants to merge 1 commit into
Open
fix: escape shell metacharacters in whisper command (command injection)#64zachariah-mithani wants to merge 1 commit into
zachariah-mithani wants to merge 1 commit into
Conversation
createCppCommand built a shell command string that was run through shelljs.exec,
interpolating user-controlled values without neutralizing shell metacharacters:
- filePath was wrapped in double quotes, but $(...) and backticks still execute
inside double quotes;
- the language whisperOption was interpolated completely unquoted (-l <value>),
so a ';' chained an arbitrary command.
Since whisper(filePath, options) is the public API and these are exactly the
caller-supplied values, untrusted input could run arbitrary commands, e.g.
whisper('$(touch pwned).wav') or whisper('a.wav', {whisperOptions:{language:'en; touch pwned'}}).
Add POSIX single-quote escaping (the package already requires a POSIX shell for
./main and make) and apply it to every user-supplied interpolation: filePath, a
custom modelPath, and the language flag. Removed the now-redundant manual double
quoting in index.ts. Normal transcriptions are unaffected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
createCppCommand(src/whisper.ts) builds a shell command string that is run throughshelljs.exec, interpolating caller-supplied values without neutralizing shell metacharacters:`./main ${getFlags(options)} -m ${modelPathOrName(...)} -f ${filePath}`filePathis wrapped in double quotes inindex.ts, but$(...)command substitution and backticks still execute inside double quotes.languagewhisper option is interpolated completely unquoted (-l+ value), so a;chains an arbitrary command.Because
whisper(filePath, options)is the public API and these are exactly the caller-supplied values, untrusted input reachingwhisper()can run arbitrary commands.Fix
Add POSIX single-quote escaping (the package already requires a POSIX shell for
./mainandmake) and apply it to every user-supplied interpolation —filePath, a custommodelPath, and thelanguageflag — and drop the now-redundant manual double-quoting inindex.ts. Normal transcriptions are unaffected.I verified that representative payloads in both the file-path and the
languageoption are rendered inert after the change, while ordinary inputs produce the same command as before. Happy to adjust the approach (e.g. switch toexecFilewith an argv array) if you'd prefer.Found with an automated taint-analysis tool (https://github.com/zachariah-mithani/security-scanner).