Skip to content
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.

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

fix: CVE-2024-21538 by migrating to promisify-child-process #4658

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
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",
);
noomorph marked this conversation as resolved.
Show resolved Hide resolved
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