fix(auth): replace exec() with execFile/spawn to prevent shell injection (#79)#98
Open
draix wants to merge 1 commit intoMiniMax-AI:mainfrom
Open
fix(auth): replace exec() with execFile/spawn to prevent shell injection (#79)#98draix wants to merge 1 commit intoMiniMax-AI:mainfrom
draix wants to merge 1 commit intoMiniMax-AI:mainfrom
Conversation
…ion (MiniMax-AI#79) src/auth/oauth.ts passed the OAuth authorization URL directly to exec(): exec(`${openCmd} "${authUrl}"`) exec() passes the string to a shell (/bin/sh on Unix, cmd.exe on Windows). A crafted authorization URL containing shell metacharacters (e.g. from a malicious or tampered authorization server redirect) could execute arbitrary commands on the user's machine. Fix: replace exec() with execFile() on macOS/Linux (which bypasses the shell entirely) and spawn() on Windows (since 'start' is a shell built-in that requires cmd.exe, spawned with shell:false and an explicit empty title arg to prevent argument injection). Closes part of MiniMax-AI#79
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
2 participants
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
Fixes the shell injection vulnerability in
src/auth/oauth.tsreported in #79.Root cause
exec()passes its argument to the system shell (/bin/shon Unix,cmd.exeon Windows). The OAuth authorization URL was passed directly toexec():A crafted
authUrlcontaining shell metacharacters (e.g. from a malicious or tampered authorization server) could execute arbitrary commands on the user's machine. Example attack vector:Fix
Replace
exec()withexecFile()/spawn(), which bypass the shell entirely and pass arguments as an array:exec('open "' + url + '"')execFile('open', [url])exec('xdg-open "' + url + '"')execFile('xdg-open', [url])exec('start "' + url + '"')spawn('cmd.exe', ['/c', 'start', '', url], { shell: false })Windows requires
cmd.exebecausestartis a shell built-in, but the URL is passed as a separate argument — never interpolated into a shell string.Part of #79
This PR addresses the Critical #1 item (shell injection). The other items (#2–#6) require larger changes (keychain integration, file locking, etc.) and are best addressed in separate PRs.