Skip to content

[APS-18613] fix: prevent command injection via cypress_config_file in loadJsFile#1080

Open
avinash-bharti wants to merge 1 commit intomasterfrom
fix/APS-18613-command-injection-cypress-config
Open

[APS-18613] fix: prevent command injection via cypress_config_file in loadJsFile#1080
avinash-bharti wants to merge 1 commit intomasterfrom
fix/APS-18613-command-injection-cypress-config

Conversation

@avinash-bharti
Copy link
Copy Markdown
Collaborator

@avinash-bharti avinash-bharti commented Apr 16, 2026

Security Fix: APS-18613

Issue

The browserstack-cypress-cli npm package contains an OS command injection vulnerability (RCE) in the cypress_config_file configuration parameter. The vulnerability exists in readCypressConfigUtil.js where the loadJsFile() function constructs a shell command by directly interpolating the user-controlled cypress_config_filepath value into a template literal and executes it via child_process.execSync().

An attacker can inject shell metacharacters (;, ", &) in the config path within browserstack.json to break out of the quoted argument and execute arbitrary commands. This was reported via HackerOne (Report #3610018).

Root Cause

The loadJsFile() function used cp.execSync() which invokes a system shell to execute the command. Because cypress_config_filepath (sourced from browserstack.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:

let load_command = `NODE_PATH=\"${bstack_node_modules_path}\" node \"${require_module_helper_path}\" \"${cypress_config_filepath}\"`
cp.execSync(load_command)

Fix Applied

Two layers of defense:

  1. Primary fix -- execFileSync instead of execSync: Replaced cp.execSync(load_command) with cp.execFileSync('node', [args], { env }). execFileSync spawns the process directly WITHOUT invoking a shell, so user-controlled values cannot break out into shell commands. NODE_PATH is passed via the env option instead of a shell prefix, which works cross-platform (Unix and Windows) without needing platform-specific branching.

  2. Defense-in-depth -- validateFilePath(): Added input validation that rejects file paths containing shell metacharacters (;, \", `, $, |, &, (, ), {, }, \\) before they reach any execution call. This guards against regression if execFileSync is ever replaced with a shell-based exec in the future.

Files Changed

  • bin/helpers/readCypressConfigUtil.js -- Security fix in loadJsFile() + new validateFilePath() function
  • test/unit/bin/helpers/readCypressConfigUtil.js -- Updated tests for execFileSync, added validateFilePath tests, added command injection rejection tests

Testing

  • Unit tests updated to verify execFileSync is called with array arguments (not string command)
  • Tests added for validateFilePath() covering: normal paths, paths with spaces, semicolon injection (Linux/macOS), ampersand injection (Windows PowerShell), backtick injection, dollar-sign injection, pipe injection
  • Tests added to confirm loadJsFile() rejects malicious file paths from the HackerOne report
  • Note: Full npm test suite execution requires local environment setup (Bash access was restricted during automated remediation). Reviewer should run npm test to confirm no regressions.

BrowserStack Session Sanity (mandatory for session repos):

  • API-verified status: N/A -- Bash access restricted in this session; session testing could not be executed
  • This fix affects local config file parsing only (before any BrowserStack session is created), so it does not impact remote session execution behavior
  • Reviewer should verify with a browserstack-cypress-cli run command using a normal browserstack.json to confirm config loading still works

Jira Ticket

APS-18613

Checklist

  • Security issue addressed (command injection eliminated via execFileSync + input validation)
  • Unit/integration tests passing (updated tests pushed; full suite run pending reviewer verification)
  • BrowserStack session run and API-verified (blocked -- Bash access restricted; fix does not affect session behavior)
  • README/docs updated if needed (no doc changes needed)

…-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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant