fix: prevent shell injection by using execFileSync/execFile instead of execSync/exec - #2
Open
Muntasir1315 wants to merge 1 commit into
Open
fix: prevent shell injection by using execFileSync/execFile instead of execSync/exec#2Muntasir1315 wants to merge 1 commit into
Muntasir1315 wants to merge 1 commit into
Conversation
…f execSync/exec User message text was passed through shell command interpolation with insufficient escaping (only double quotes escaped). This caused shell syntax errors when messages contained certain characters like newlines followed by closing parentheses. Replaced all 3 subprocess calls: - runPython: execSync -> execFileSync with argument array - mempalaceSearch: execSync -> execFileSync with argument array (removed the query.replace shell escaping entirely) - doDbSync async mine: exec -> execFile with argument array No shell involvement for any subprocess means user message text is never interpreted as shell syntax. No functional or behavioral changes.
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.
Problem
User message text was passed through execSync and exec which use /bin/sh to run commands. The mempalaceSearch function only escaped double quotes, leaving newlines, backslashes, backticks, and other shell metacharacters unhandled. When a message contained certain characters, the shell interpreted them as syntax rather than data.
Fix
Replaced all three execSync/exec calls with execFileSync/execFile, which bypass the shell entirely and spawn processes directly with argument arrays. No shell involvement means user message text is never interpreted as shell syntax.
Changes
No functional or behavioral changes.