Skip to content

Commit

Permalink
fix: CVE-2024-21538 by migrating to promisify-child-process
Browse files Browse the repository at this point in the history
fixes: #4657 and #4640
  • Loading branch information
matin-zadeh-dolatabad committed Dec 13, 2024
1 parent 4d554df commit 828716c
Show file tree
Hide file tree
Showing 14 changed files with 120 additions and 54 deletions.
3 changes: 1 addition & 2 deletions detox/local-cli/utils/frameworkUtils.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
const os = require('os');
const path = require('path');

const { spawn } = require('child-process-promise');
const fs = require('fs-extra');
const { spawn } = require('promisify-child-process');

const detox = require('../../internals');
const { getFrameworkDirPath, getXCUITestRunnerDirPath } = require('../../src/utils/environment');


const frameworkBuildScript = '../../scripts/build_local_framework.ios.sh';
const xcuitestBuildScript = '../../scripts/build_local_xcuitest.ios.sh';

Expand Down
2 changes: 1 addition & 1 deletion detox/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@
"bunyan-debug-stream": "^3.1.0",
"caf": "^15.0.1",
"chalk": "^4.0.0",
"child-process-promise": "^2.2.0",
"detox-copilot": "^0.0.24",
"execa": "^5.1.1",
"find-up": "^5.0.0",
"fs-extra": "^11.0.0",
"funpermaproxy": "^1.1.0",
"glob": "^8.0.3",
"ini": "^1.3.4",
"promisify-child-process": "^4.1.2",
"jest-environment-emit": "^1.0.8",
"json-cycle": "^1.3.0",
"lodash": "^4.17.11",
Expand Down
13 changes: 9 additions & 4 deletions detox/src/devices/common/drivers/android/exec/BinaryExec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// @ts-nocheck
const spawn = require('child-process-promise').spawn;
const { spawn } = require('promisify-child-process');

const exec = require('../../../../../utils/childProcess').execWithRetriesAndLogs;

Expand Down Expand Up @@ -27,7 +26,13 @@ class BinaryExec {
}

async exec(command) {
return (await exec(`"${this.binary}" ${command._getArgsString()}`)).stdout;
const result = await exec(`"${this.binary}" ${command._getArgsString()}`);

if (!result) {
throw new Error(`Failed to execute binary: ${this.binary}`);
}

return result.stdout;
}

spawn(command, stdout, stderr) {
Expand All @@ -37,5 +42,5 @@ class BinaryExec {

module.exports = {
ExecCommand,
BinaryExec,
BinaryExec
};
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable node/no-extraneous-require */
describe('BinaryExec', () => {
const binaryPath = '/binary/mock';

Expand All @@ -12,10 +13,10 @@ describe('BinaryExec', () => {
}));
exec = require('../../../../../utils/childProcess').execWithRetriesAndLogs;

jest.mock('child-process-promise', () => ({
jest.mock('promisify-child-process', () => ({
spawn: jest.fn()
}));
spawn = require('child-process-promise').spawn;
spawn = require('promisify-child-process').spawn;

const { BinaryExec } = require('./BinaryExec');
binaryExec = new BinaryExec(binaryPath);
Expand Down Expand Up @@ -87,7 +88,7 @@ describe('BinaryExec', () => {
expect(spawn).toHaveBeenCalledWith(binaryPath, commandArgs, expect.anything());
});

it('should chain-return child-process-promise from spawn', async () => {
it('should chain-return promisify-child-process from spawn', async () => {
const childProcessPromise = Promise.resolve('mock result');
spawn.mockReturnValue(childProcessPromise);

Expand Down
14 changes: 7 additions & 7 deletions detox/src/devices/runtime/drivers/ios/SimulatorDriver.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// @ts-nocheck
const path = require('path');

const exec = require('child-process-promise').exec;
const _ = require('lodash');
const { exec } = require('promisify-child-process');

const temporaryPath = require('../../../../artifacts/utils/temporaryPath');
const DetoxRuntimeError = require('../../../../errors/DetoxRuntimeError');
Expand All @@ -16,7 +16,6 @@ const traceInvocationCall = require('../../../../utils/traceInvocationCall').bin

const IosDriver = require('./IosDriver');


/**
* @typedef SimulatorDriverDeps { DeviceDriverDeps }
* @property applesimutils { AppleSimUtils }
Expand Down Expand Up @@ -111,8 +110,9 @@ class SimulatorDriver extends IosDriver {
if (Number.isNaN(pid)) {
throw new DetoxRuntimeError({
message: `Failed to find a process corresponding to the app bundle identifier (${bundleId}).`,
hint: `Make sure that the app is running on the device (${udid}), visually or via CLI:\n` +
`xcrun simctl spawn ${this.udid} launchctl list | grep -F '${bundleId}'\n`,
hint:
`Make sure that the app is running on the device (${udid}), visually or via CLI:\n` +
`xcrun simctl spawn ${this.udid} launchctl list | grep -F '${bundleId}'\n`
});
} else {
log.info({}, `Found the app (${bundleId}) with process ID = ${pid}. Proceeding...`);
Expand Down Expand Up @@ -141,7 +141,7 @@ class SimulatorDriver extends IosDriver {
const xcuitestRunner = new XCUITestRunner({ runtimeDevice: { id: this.getExternalId(), _bundleId } });
let x = point?.x ?? 100;
let y = point?.y ?? 100;
let _pressDuration = pressDuration ? (pressDuration / 1000) : 1;
let _pressDuration = pressDuration ? pressDuration / 1000 : 1;
const traceDescription = actionDescription.longPress({ x, y }, _pressDuration);
return this.withAction(xcuitestRunner, 'coordinateLongPress', traceDescription, x.toString(), y.toString(), _pressDuration.toString());
}
Expand Down Expand Up @@ -210,7 +210,7 @@ class SimulatorDriver extends IosDriver {
await this.emitter.emit('createExternalArtifact', {
pluginId: 'screenshot',
artifactName: screenshotName || path.basename(tempPath, '.png'),
artifactPath: tempPath,
artifactPath: tempPath
});

return tempPath;
Expand All @@ -223,7 +223,7 @@ class SimulatorDriver extends IosDriver {
await this.emitter.emit('createExternalArtifact', {
pluginId: 'uiHierarchy',
artifactName: artifactName,
artifactPath: viewHierarchyURL,
artifactPath: viewHierarchyURL
});

return viewHierarchyURL;
Expand Down
2 changes: 1 addition & 1 deletion detox/src/ios/XCUITestRunner.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { exec } = require('child-process-promise');
const { exec } = require('promisify-child-process');

const DetoxRuntimeError = require('../errors/DetoxRuntimeError');
const environment = require('../utils/environment');
Expand Down
4 changes: 2 additions & 2 deletions detox/src/ios/XCUITestRunner.test.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
const XCUITestRunner = require('./XCUITestRunner');

jest.mock('child-process-promise', () => {
jest.mock('promisify-child-process', () => {
return {
exec: jest.fn(),
};
});

const { exec } = jest.requireMock('child-process-promise');
const { exec } = jest.requireMock('promisify-child-process');
const environment = jest.requireMock('../utils/environment');

jest.mock('../utils/environment');
Expand Down
8 changes: 7 additions & 1 deletion detox/src/utils/childProcess/exec.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
// @ts-nocheck
const { exec } = require('child-process-promise');
const _ = require('lodash');
const { exec } = require('promisify-child-process');

const DetoxRuntimeError = require('../../errors/DetoxRuntimeError');
const rootLogger = require('../logger').child({ cat: ['child-process', 'child-process-exec'] });
const retry = require('../retry');

const execsCounter = require('./opsCounter');

/**
*
* @param {*} bin
* @param {*} options
* @returns {Promise<import('promisify-child-process').Output>}
*/
async function execWithRetriesAndLogs(bin, options = {}) {
const {
retries = 9,
Expand Down
36 changes: 18 additions & 18 deletions detox/src/utils/childProcess/exec.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ describe('Exec utils', () => {
jest.mock('../logger');
logger = require('../logger');

jest.mock('child-process-promise');
cpp = require('child-process-promise');
jest.mock('promisify-child-process');
cpp = require('promisify-child-process');

exec = require('./exec');
});
Expand Down Expand Up @@ -91,8 +91,8 @@ describe('Exec utils', () => {
args: `--argument 123`,
statusLogs: {
trying: 'trying status log',
successful: 'successful status log',
},
successful: 'successful status log'
}
};
await execWithRetriesAndLogs('bin', options);

Expand All @@ -110,8 +110,8 @@ describe('Exec utils', () => {
const options = {
args: `--argument 123`,
statusLogs: {
retrying: true,
},
retrying: true
}
};

logger.debug.mockClear();
Expand Down Expand Up @@ -233,20 +233,20 @@ describe('Exec utils', () => {
});

const returnSuccessfulWithValue = (value) => ({
stdout: JSON.stringify(value),
stderr: 'err',
childProcess: {
exitCode: 0
}
});
stdout: JSON.stringify(value),
stderr: 'err',
childProcess: {
exitCode: 0
}
});

const returnErrorWithValue = (value) => ({
stdout: 'out',
stderr: value,
childProcess: {
exitCode: 1
}
});
stdout: 'out',
stderr: value,
childProcess: {
exitCode: 1
}
});

function mockCppSuccessful(cpp) {
const successfulResult = returnSuccessfulWithValue('successful result');
Expand Down
2 changes: 1 addition & 1 deletion detox/src/utils/childProcess/spawn.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @ts-nocheck
const { spawn } = require('child-process-promise');
const _ = require('lodash');
const { spawn } = require('promisify-child-process');

const rootLogger = require('../logger').child({ cat: ['child-process', 'child-process-spawn'] });
const { escape } = require('../pipeCommands');
Expand Down
4 changes: 2 additions & 2 deletions detox/src/utils/childProcess/spawn.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ describe('Spawn utils', () => {
jest.mock('../logger');
log = require('../logger');

jest.mock('child-process-promise');
cpp = require('child-process-promise');
jest.mock('promisify-child-process');
cpp = require('promisify-child-process');

jest.mock('../retry');
retry = require('../retry');
Expand Down
35 changes: 23 additions & 12 deletions detox/src/utils/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ const fs = require('fs');
const os = require('os');
const path = require('path');

const exec = require('child-process-promise').exec;
const ini = require('ini');
const _ = require('lodash');
const { exec } = require('promisify-child-process');
const _which = require('which');

const DetoxRuntimeError = require('../errors/DetoxRuntimeError');
Expand Down Expand Up @@ -147,8 +147,7 @@ function throwMissingAvdINIError(avdName, avdIniPath) {

function throwMissingAvdError(avdName, avdPath, avdIniPath) {
throw new DetoxRuntimeError(
`Failed to find AVD ${avdName} directory at path: ${avdPath}\n` +
`Please verify "path" property in the INI file: ${avdIniPath}`
`Failed to find AVD ${avdName} directory at path: ${avdPath}\n` + `Please verify "path" property in the INI file: ${avdIniPath}`
);
}

Expand All @@ -169,7 +168,9 @@ function throwSdkIntegrityError(errMessage) {
}

function throwMissingGmsaasError() {
throw new DetoxRuntimeError(`Failed to locate Genymotion's gmsaas executable. Please add it to your $PATH variable!\nPATH is currently set to: ${process.env.PATH}`);
throw new DetoxRuntimeError(
`Failed to locate Genymotion's gmsaas executable. Please add it to your $PATH variable!\nPATH is currently set to: ${process.env.PATH}`
);
}

const getDetoxVersion = _.once(() => {
Expand All @@ -178,11 +179,15 @@ const getDetoxVersion = _.once(() => {

const getBuildFolderName = _.once(async () => {
const detoxVersion = getDetoxVersion();
const xcodeVersion = await exec('xcodebuild -version').then(result => result.stdout.trim());

return crypto.createHash('sha1')
.update(`${detoxVersion}\n${xcodeVersion}\n`)
.digest('hex');
const xcodeVersion = await exec('xcodebuild -version').then((result) => {
if (typeof result.stdout === 'string') {
return result.stdout.trim();
} else {
throw new DetoxRuntimeError(`Failed trim stdout result of command: xcodebuild -version`);
}
});

return crypto.createHash('sha1').update(`${detoxVersion}\n${xcodeVersion}\n`).digest('hex');
});

const getFrameworkDirPath = `${DETOX_LIBRARY_ROOT_PATH}/ios/framework`;
Expand All @@ -197,8 +202,14 @@ const getXCUITestRunnerDirPath = `${DETOX_LIBRARY_ROOT_PATH}/ios/xcuitest-runner
const getXCUITestRunnerPath = _.once(async () => {
const buildFolder = await getBuildFolderName();
const derivedDataPath = `${getXCUITestRunnerDirPath}/${buildFolder}`;
const xctestrunPath = await exec(`find ${derivedDataPath} -name "*.xctestrun" -print -quit`)
.then(result => result.stdout.trim());
const command = `find ${derivedDataPath} -name "*.xctestrun" -print -quit`;
const xctestrunPath = await exec(command).then((result) => {
if (typeof result.stdout === 'string') {
return result.stdout.trim();
} else {
throw new DetoxRuntimeError(`Failed trim stdout result of command: ${command}`);
}
});

if (!xctestrunPath) {
throw new DetoxRuntimeError(`Failed to find .xctestrun file in ${derivedDataPath}`);
Expand Down Expand Up @@ -246,5 +257,5 @@ module.exports = {
getDetoxLockFilePath,
getDeviceRegistryPath,
getLastFailedTestsPath,
getHomeDir,
getHomeDir
};
22 changes: 22 additions & 0 deletions detox/test/ios/example.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,17 @@
GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
GCC_WARN_UNUSED_FUNCTION = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
HEADER_SEARCH_PATHS = (
"$(inherited)",
"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers",
"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core",
"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon-Samples/ReactCommon_Samples.framework/Headers",
"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon-Samples/ReactCommon_Samples.framework/Headers/platform/ios",
"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers/react/renderer/components/view/platform/cxx",
"${PODS_CONFIGURATION_BUILD_DIR}/React-NativeModulesApple/React_NativeModulesApple.framework/Headers",
"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers",
"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios",
);
IPHONEOS_DEPLOYMENT_TARGET = 13.0;
LD = "";
LDPLUSPLUS = "";
Expand Down Expand Up @@ -871,6 +882,17 @@
GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
GCC_WARN_UNUSED_FUNCTION = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
HEADER_SEARCH_PATHS = (
"$(inherited)",
"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers",
"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core",
"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon-Samples/ReactCommon_Samples.framework/Headers",
"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon-Samples/ReactCommon_Samples.framework/Headers/platform/ios",
"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers/react/renderer/components/view/platform/cxx",
"${PODS_CONFIGURATION_BUILD_DIR}/React-NativeModulesApple/React_NativeModulesApple.framework/Headers",
"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers",
"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios",
);
IPHONEOS_DEPLOYMENT_TARGET = 13.0;
LD = "";
LDPLUSPLUS = "";
Expand Down
Loading

0 comments on commit 828716c

Please sign in to comment.