From a3b5b1a7d5d1e39321bc147cd92515068a17e856 Mon Sep 17 00:00:00 2001 From: Giovanni Date: Thu, 5 Dec 2024 17:45:22 +0100 Subject: [PATCH] child_process: validate strings in exec and spawn --- lib/child_process.js | 18 +++++---- .../parallel/test-child-process-exec-error.js | 40 +++++++++++++++++-- test/parallel/test-child-process-execfile.js | 30 ++++++++++++++ .../test-child-process-spawn-error.js | 16 ++++++++ test/parallel/test-child-process-spawnsync.js | 15 +++++++ 5 files changed, 107 insertions(+), 12 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 3fb21f755be3d7..99e929857fb180 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -186,9 +186,6 @@ function _forkChild(fd, serializationMode) { } function normalizeExecArgs(command, options, callback) { - validateString(command, 'command'); - validateArgumentNullCheck(command, 'command'); - if (typeof options === 'function') { callback = options; options = undefined; @@ -535,12 +532,17 @@ function copyProcessEnvToEnv(env, name, optionEnv) { } } -function normalizeSpawnArguments(file, args, options) { - validateString(file, 'file'); - validateArgumentNullCheck(file, 'file'); +function validateStringParam(param, paramName) { + validateString(param, paramName); + validateArgumentNullCheck(param, paramName); + + if (param.length === 0) { + throw new ERR_INVALID_ARG_VALUE(paramName, param, 'cannot be empty'); + } +} - if (file.length === 0) - throw new ERR_INVALID_ARG_VALUE('file', file, 'cannot be empty'); +function normalizeSpawnArguments(file, args, options) { + validateStringParam(file, 'file'); if (ArrayIsArray(args)) { args = ArrayPrototypeSlice(args); diff --git a/test/parallel/test-child-process-exec-error.js b/test/parallel/test-child-process-exec-error.js index cd45f3071c2920..59e661171556ca 100644 --- a/test/parallel/test-child-process-exec-error.js +++ b/test/parallel/test-child-process-exec-error.js @@ -22,7 +22,7 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); -const child_process = require('child_process'); +const { exec, execSync, execFile } = require('child_process'); function test(fn, code, expectPidType = 'number') { const child = fn('does-not-exist', common.mustCall(function(err) { @@ -35,10 +35,42 @@ function test(fn, code, expectPidType = 'number') { // With `shell: true`, expect pid (of the shell) if (common.isWindows) { - test(child_process.exec, 1, 'number'); // Exit code of cmd.exe + test(exec, 1, 'number'); // Exit code of cmd.exe } else { - test(child_process.exec, 127, 'number'); // Exit code of /bin/sh + test(exec, 127, 'number'); // Exit code of /bin/sh } // With `shell: false`, expect no pid -test(child_process.execFile, 'ENOENT', 'undefined'); +test(execFile, 'ENOENT', 'undefined'); + + +// Verify that the exec() function throws when command parameter is not a valid string +{ + assert.throws(() => { + exec(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + exec('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + exec('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} + + +// Verify that the execSync() function throws when command parameter is not a valid string +{ + assert.throws(() => { + execSync(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + execSync('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + execSync('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} diff --git a/test/parallel/test-child-process-execfile.js b/test/parallel/test-child-process-execfile.js index c4dba6b3f9466f..f1833eaa88d9c9 100644 --- a/test/parallel/test-child-process-execfile.js +++ b/test/parallel/test-child-process-execfile.js @@ -125,3 +125,33 @@ const execOpts = { encoding: 'utf8', shell: true, env: { ...process.env, NODE: p })); }); } + +// Verify that the execFile() function throws when the file parameter is not a valid string +{ + assert.throws(() => { + execFile(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + execFile('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + execFile('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} + +// Verify that the execFileSync() function throws when file parameter is not a valid string +{ + assert.throws(() => { + execFileSync(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + execFileSync('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + execFileSync('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} diff --git a/test/parallel/test-child-process-spawn-error.js b/test/parallel/test-child-process-spawn-error.js index ed1c8fac9f97ed..0885ed52d796d4 100644 --- a/test/parallel/test-child-process-spawn-error.js +++ b/test/parallel/test-child-process-spawn-error.js @@ -53,3 +53,19 @@ enoentChild.on('error', common.mustCall(function(err) { assert.strictEqual(err.path, enoentPath); assert.deepStrictEqual(err.spawnargs, spawnargs); })); + + +// Verify that the spawn() function throws when the file parameter is not a valid string +{ + assert.throws(() => { + spawn(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + spawn('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + spawn('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +} diff --git a/test/parallel/test-child-process-spawnsync.js b/test/parallel/test-child-process-spawnsync.js index 9ec125ea891689..5d89af35db0de1 100644 --- a/test/parallel/test-child-process-spawnsync.js +++ b/test/parallel/test-child-process-spawnsync.js @@ -65,3 +65,18 @@ assert.deepStrictEqual(ret_err.spawnargs, ['bar']); ]; assert.deepStrictEqual(retUTF8.output, stringifiedDefault); } + +// Verify that the spawnSync() function throws when the file parameter is not a valid string +{ + assert.throws(() => { + spawnSync(123, common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_TYPE' }); + + assert.throws(() => { + spawnSync('', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); + + assert.throws(() => { + spawnSync('\u0000', common.mustNotCall()); + }, { code: 'ERR_INVALID_ARG_VALUE' }); +}