diff options
author | Sakthipriyan Vairamani (thefourtheye) <thechargingvolcano@gmail.com> | 2016-12-23 14:36:20 +0530 |
---|---|---|
committer | Sakthipriyan Vairamani (thefourtheye) <thechargingvolcano@gmail.com> | 2017-04-04 13:02:51 +0530 |
commit | d75fdd96aac39f5432777a9763e7290419d401bd (patch) | |
tree | 2164afc37003cb8211fe2f2735285fb18f380c0f /lib | |
parent | a5f91ab230c574d561780b6867d00f06fcc1e4de (diff) | |
download | android-node-v8-d75fdd96aac39f5432777a9763e7290419d401bd.tar.gz android-node-v8-d75fdd96aac39f5432777a9763e7290419d401bd.tar.bz2 android-node-v8-d75fdd96aac39f5432777a9763e7290419d401bd.zip |
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>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/child_process.js | 30 | ||||
-rw-r--r-- | lib/internal/child_process.js | 17 | ||||
-rw-r--r-- | lib/internal/util.js | 26 |
3 files changed, 36 insertions, 37 deletions
diff --git a/lib/child_process.js b/lib/child_process.js index c2811109b7..34e420eada 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -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; } diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 104ac4f636..05e1cd63b0 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -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); diff --git a/lib/internal/util.js b/lib/internal/util.js index 3de57040f9..7bf92566d8 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -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); +}; |