[APS-18613] fix: prevent command injection via cypress_config_file in loadJsFile#1080
Open
avinash-bharti wants to merge 1 commit intomasterfrom
Open
[APS-18613] fix: prevent command injection via cypress_config_file in loadJsFile#1080avinash-bharti wants to merge 1 commit intomasterfrom
avinash-bharti wants to merge 1 commit intomasterfrom
Conversation
…-18613] - Replace execSync() with execFileSync() in loadJsFile() to avoid shell interpolation of user-controlled cypress_config_filepath values - Pass NODE_PATH via env option instead of shell command prefix, which works cross-platform (Unix and Windows) without shell metacharacters - Add validateFilePath() defense-in-depth check that rejects paths containing shell metacharacters before they reach any exec call - Update unit tests to verify execFileSync is called with array args and to confirm command injection payloads are rejected Resolves: APS-18613
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.
Security Fix: APS-18613
Issue
The
browserstack-cypress-clinpm package contains an OS command injection vulnerability (RCE) in thecypress_config_fileconfiguration parameter. The vulnerability exists inreadCypressConfigUtil.jswhere theloadJsFile()function constructs a shell command by directly interpolating the user-controlledcypress_config_filepathvalue into a template literal and executes it viachild_process.execSync().An attacker can inject shell metacharacters (
;,",&) in the config path withinbrowserstack.jsonto break out of the quoted argument and execute arbitrary commands. This was reported via HackerOne (Report #3610018).Root Cause
The
loadJsFile()function usedcp.execSync()which invokes a system shell to execute the command. Becausecypress_config_filepath(sourced frombrowserstack.json) was interpolated directly into the command string without sanitization, shell metacharacters in the path were interpreted by the shell, enabling arbitrary command execution.Vulnerable code:
Fix Applied
Two layers of defense:
Primary fix --
execFileSyncinstead ofexecSync: Replacedcp.execSync(load_command)withcp.execFileSync('node', [args], { env }).execFileSyncspawns the process directly WITHOUT invoking a shell, so user-controlled values cannot break out into shell commands.NODE_PATHis passed via theenvoption instead of a shell prefix, which works cross-platform (Unix and Windows) without needing platform-specific branching.Defense-in-depth --
validateFilePath(): Added input validation that rejects file paths containing shell metacharacters (;,\",`,$,|,&,(,),{,},\\) before they reach any execution call. This guards against regression ifexecFileSyncis ever replaced with a shell-based exec in the future.Files Changed
bin/helpers/readCypressConfigUtil.js-- Security fix inloadJsFile()+ newvalidateFilePath()functiontest/unit/bin/helpers/readCypressConfigUtil.js-- Updated tests forexecFileSync, addedvalidateFilePathtests, added command injection rejection testsTesting
execFileSyncis called with array arguments (not string command)validateFilePath()covering: normal paths, paths with spaces, semicolon injection (Linux/macOS), ampersand injection (Windows PowerShell), backtick injection, dollar-sign injection, pipe injectionloadJsFile()rejects malicious file paths from the HackerOne reportnpm testsuite execution requires local environment setup (Bash access was restricted during automated remediation). Reviewer should runnpm testto confirm no regressions.BrowserStack Session Sanity (mandatory for session repos):
browserstack-cypress-cli runcommand using a normalbrowserstack.jsonto confirm config loading still worksJira Ticket
APS-18613
Checklist