child_process: improve killSignal validations
As it is, the `killSignal` is just retrieved from an object and used. If the signal passed is actually one of the inherited properties of that object, Node.js will die. For example, ➜ node -e "child_process.spawnSync('ls', {killSignal: 'toString'})" Assertion failed: (0), function uv_close, file ....core.c, line 166. [1] 58938 abort node -e "child_process.spawnSync(...)" 1. This patch makes sure that the signal is actually a own property of the constants object. 2. Extends the killSignal validation to all the other functions. PR-URL: https://github.com/nodejs/node/pull/10423 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit is contained in:
parent
a5f91ab230
commit
d75fdd96aa
@ -22,9 +22,8 @@
|
||||
'use strict';
|
||||
|
||||
const util = require('util');
|
||||
const internalUtil = require('internal/util');
|
||||
const { deprecate, convertToValidSignal } = require('internal/util');
|
||||
const debug = util.debuglog('child_process');
|
||||
const constants = process.binding('constants').os.signals;
|
||||
|
||||
const uv = process.binding('uv');
|
||||
const spawn_sync = process.binding('spawn_sync');
|
||||
@ -181,6 +180,8 @@ exports.execFile = function(file /*, args, options, callback*/) {
|
||||
// Validate maxBuffer, if present.
|
||||
validateMaxBuffer(options.maxBuffer);
|
||||
|
||||
options.killSignal = sanitizeKillSignal(options.killSignal);
|
||||
|
||||
var child = spawn(file, args, {
|
||||
cwd: options.cwd,
|
||||
env: options.env,
|
||||
@ -332,7 +333,7 @@ exports.execFile = function(file /*, args, options, callback*/) {
|
||||
return child;
|
||||
};
|
||||
|
||||
const _deprecatedCustomFds = internalUtil.deprecate(
|
||||
const _deprecatedCustomFds = deprecate(
|
||||
function deprecateCustomFds(options) {
|
||||
options.stdio = options.customFds.map(function mapCustomFds(fd) {
|
||||
return fd === -1 ? 'pipe' : fd;
|
||||
@ -474,18 +475,6 @@ var spawn = exports.spawn = function(/*file, args, options*/) {
|
||||
return child;
|
||||
};
|
||||
|
||||
|
||||
function lookupSignal(signal) {
|
||||
if (typeof signal === 'number')
|
||||
return signal;
|
||||
|
||||
if (!(signal in constants))
|
||||
throw new Error('Unknown signal: ' + signal);
|
||||
|
||||
return constants[signal];
|
||||
}
|
||||
|
||||
|
||||
function spawnSync(/*file, args, options*/) {
|
||||
var opts = normalizeSpawnArguments.apply(null, arguments);
|
||||
|
||||
@ -506,7 +495,7 @@ function spawnSync(/*file, args, options*/) {
|
||||
options.envPairs = opts.envPairs;
|
||||
|
||||
// Validate and translate the kill signal, if present.
|
||||
options.killSignal = validateKillSignal(options.killSignal);
|
||||
options.killSignal = sanitizeKillSignal(options.killSignal);
|
||||
|
||||
options.stdio = _validateStdio(options.stdio || 'pipe', true).stdio;
|
||||
|
||||
@ -632,15 +621,10 @@ function validateMaxBuffer(maxBuffer) {
|
||||
}
|
||||
|
||||
|
||||
function validateKillSignal(killSignal) {
|
||||
function sanitizeKillSignal(killSignal) {
|
||||
if (typeof killSignal === 'string' || typeof killSignal === 'number') {
|
||||
killSignal = lookupSignal(killSignal);
|
||||
|
||||
if (killSignal === 0)
|
||||
throw new RangeError('"killSignal" cannot be 0');
|
||||
return convertToValidSignal(killSignal);
|
||||
} else if (killSignal != null) {
|
||||
throw new TypeError('"killSignal" must be a string or number');
|
||||
}
|
||||
|
||||
return killSignal;
|
||||
}
|
||||
|
@ -5,7 +5,6 @@ const EventEmitter = require('events');
|
||||
const net = require('net');
|
||||
const dgram = require('dgram');
|
||||
const util = require('util');
|
||||
const constants = process.binding('constants').os.signals;
|
||||
const assert = require('assert');
|
||||
|
||||
const Process = process.binding('process_wrap').Process;
|
||||
@ -17,6 +16,7 @@ const TCP = process.binding('tcp_wrap').TCP;
|
||||
const UDP = process.binding('udp_wrap').UDP;
|
||||
const SocketList = require('internal/socket_list');
|
||||
const { isUint8Array } = process.binding('util');
|
||||
const { convertToValidSignal } = require('internal/util');
|
||||
|
||||
const errnoException = util._errnoException;
|
||||
const SocketListSend = SocketList.SocketListSend;
|
||||
@ -362,19 +362,8 @@ function onErrorNT(self, err) {
|
||||
|
||||
|
||||
ChildProcess.prototype.kill = function(sig) {
|
||||
var signal;
|
||||
|
||||
if (sig === 0) {
|
||||
signal = 0;
|
||||
} else if (!sig) {
|
||||
signal = constants['SIGTERM'];
|
||||
} else {
|
||||
signal = constants[sig];
|
||||
}
|
||||
|
||||
if (signal === undefined) {
|
||||
throw new Error('Unknown signal: ' + sig);
|
||||
}
|
||||
const signal = sig === 0 ? sig :
|
||||
convertToValidSignal(sig === undefined ? 'SIGTERM' : sig);
|
||||
|
||||
if (this._handle) {
|
||||
var err = this._handle.kill(signal);
|
||||
|
@ -1,6 +1,7 @@
|
||||
'use strict';
|
||||
|
||||
const binding = process.binding('util');
|
||||
const signals = process.binding('constants').os.signals;
|
||||
|
||||
const kArrowMessagePrivateSymbolIndex = binding['arrow_message_private_symbol'];
|
||||
const kDecoratedPrivateSymbolIndex = binding['decorated_private_symbol'];
|
||||
@ -179,3 +180,28 @@ exports.createClassWrapper = function createClassWrapper(type) {
|
||||
fn.prototype = type.prototype;
|
||||
return fn;
|
||||
};
|
||||
|
||||
let signalsToNamesMapping;
|
||||
function getSignalsToNamesMapping() {
|
||||
if (signalsToNamesMapping !== undefined)
|
||||
return signalsToNamesMapping;
|
||||
|
||||
signalsToNamesMapping = Object.create(null);
|
||||
for (const key in signals) {
|
||||
signalsToNamesMapping[signals[key]] = key;
|
||||
}
|
||||
|
||||
return signalsToNamesMapping;
|
||||
}
|
||||
|
||||
exports.convertToValidSignal = function convertToValidSignal(signal) {
|
||||
if (typeof signal === 'number' && getSignalsToNamesMapping()[signal])
|
||||
return signal;
|
||||
|
||||
if (typeof signal === 'string') {
|
||||
const signalName = signals[signal.toUpperCase()];
|
||||
if (signalName) return signalName;
|
||||
}
|
||||
|
||||
throw new Error('Unknown signal: ' + signal);
|
||||
};
|
||||
|
@ -2,6 +2,7 @@
|
||||
const common = require('../common');
|
||||
const assert = require('assert');
|
||||
const spawnSync = require('child_process').spawnSync;
|
||||
const signals = process.binding('constants').os.signals;
|
||||
|
||||
function pass(option, value) {
|
||||
// Run the command with the specified option. Since it's not a real command,
|
||||
@ -184,18 +185,32 @@ if (!common.isWindows) {
|
||||
{
|
||||
// Validate the killSignal option
|
||||
const typeErr = /^TypeError: "killSignal" must be a string or number$/;
|
||||
const rangeErr = /^RangeError: "killSignal" cannot be 0$/;
|
||||
const unknownSignalErr = /^Error: Unknown signal:/;
|
||||
|
||||
pass('killSignal', undefined);
|
||||
pass('killSignal', null);
|
||||
pass('killSignal', 'SIGKILL');
|
||||
pass('killSignal', 500);
|
||||
fail('killSignal', 0, rangeErr);
|
||||
fail('killSignal', 'SIGNOTAVALIDSIGNALNAME', unknownSignalErr);
|
||||
fail('killSignal', true, typeErr);
|
||||
fail('killSignal', false, typeErr);
|
||||
fail('killSignal', [], typeErr);
|
||||
fail('killSignal', {}, typeErr);
|
||||
fail('killSignal', common.noop, typeErr);
|
||||
|
||||
// Invalid signal names and numbers should fail
|
||||
fail('killSignal', 500, unknownSignalErr);
|
||||
fail('killSignal', 0, unknownSignalErr);
|
||||
fail('killSignal', -200, unknownSignalErr);
|
||||
fail('killSignal', 3.14, unknownSignalErr);
|
||||
|
||||
Object.getOwnPropertyNames(Object.prototype).forEach((property) => {
|
||||
fail('killSignal', property, unknownSignalErr);
|
||||
});
|
||||
|
||||
// Valid signal names and numbers should pass
|
||||
for (const signalName in signals) {
|
||||
pass('killSignal', signals[signalName]);
|
||||
pass('killSignal', signalName);
|
||||
pass('killSignal', signalName.toLowerCase());
|
||||
}
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user