fix(javascript-lang-security-detect-child-process-detect-child-process): Shell Injection via Unquoted Build Parameters in xcodebuild Command - #32
Open
zepto-gaurav wants to merge 1 commit into
Conversation
…s): Shell Injection via Unquoted Build Parameters in xcodebuild Command Fixes 1 finding(s): 00e78716 Auto-generated by Optimus AutoFix Agent
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
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.
Optimus AutoFix — Automated Security Fix
Findings Fixed
Fix Strategy
Shell Injection via Unquoted Build Parameters in xcodebuild Command
Changes Made
execSyncwith shell string interpolation withexecFileSync(argument array form) to eliminate shell interpretation entirely. This is the correct fix for the root cause: the shell is never invoked, so no parameter — scheme, destination, buildFolder, configuration, or rootFolder — can ever be used to inject shell metacharacters. Before making changes, read the full file to confirm the existingrequire('child_process')import and check whetherexecSyncis used anywhere else in the file.Specific changes:
require('child_process')destructuring at the top of the file to importexecFileSyncinstead of (or in addition to, ifexecSyncis used elsewhere)execSync.buildPlatform, delete thecommandstring construction entirely.execSync(command, {cwd: rootFolder, ...})call with:This covers ALL five interpolated values (scheme, destination, buildFolder, configuration, rootFolder) because none of them are ever passed through a shell. The fix is future-safe: any new arguments added to this invocation in the same array form will also bypass the shell.
AI Review
execSync(shell-based) withexecFileSync(shell-bypassing argument array form). All five previously interpolated values —scheme,destination,buildFolder,configuration, androotFolder— are now handled safely: the first four are passed as discrete array elements androotFolderis passed only as thecwdoption, never reaching a shell. The import is properly updated and no residualexecSyncreferences remain. The fix is minimal, syntactically correct, and preserves the exact behavioral semantics of the original invocation.Issues
destinationvalue is still embedded in a JavaScript template literal (generic/platform=${destination}). This is safe because the string is an array element passed directly to the OS without shell interpretation — but a code comment explaining this distinction could help future maintainers understand why the template literal here is not a concern, unlike the original shell-interpolated form.Risk Assessment
Auto-generated by Optimus AutoFix Agent. Review carefully before merging.