From c842094c3bb08388b73f3efc0923280ae20ff767 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Wed, 29 Aug 2018 02:48:37 +0300 Subject: [PATCH 1/2] child_process: fix handling of incorrect uid/gid in spawn uid/gid must be uint32, which is asserted on a c++ side but wasn't checked on a JS side and therefore resulted in a process crash. Refs: https://github.com/nodejs/node/issues/22570 --- lib/child_process.js | 12 +++++++----- test/parallel/test-child-process-spawn-typeerror.js | 10 +++++++++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 1ab4150b01e659..214f41f2c36dbf 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -37,7 +37,7 @@ const { ERR_INVALID_OPT_VALUE, ERR_OUT_OF_RANGE } = require('internal/errors').codes; -const { validateString } = require('internal/validators'); +const { validateString, isUint32 } = require('internal/validators'); const child_process = require('internal/child_process'); const { _validateStdio, @@ -425,13 +425,15 @@ function normalizeSpawnArguments(file, args, options) { } // Validate the uid, if present. - if (options.uid != null && !Number.isInteger(options.uid)) { - throw new ERR_INVALID_ARG_TYPE('options.uid', 'integer', options.uid); + if (options.uid != null && + (!Number.isInteger(options.uid) || !isUint32(options.uid))) { + throw new ERR_INVALID_ARG_TYPE('options.uid', 'uint32', options.uid); } // Validate the gid, if present. - if (options.gid != null && !Number.isInteger(options.gid)) { - throw new ERR_INVALID_ARG_TYPE('options.gid', 'integer', options.gid); + if (options.gid != null && + (!Number.isInteger(options.gid) || !isUint32(options.gid))) { + throw new ERR_INVALID_ARG_TYPE('options.gid', 'uint32', options.gid); } // Validate the shell, if present. diff --git a/test/parallel/test-child-process-spawn-typeerror.js b/test/parallel/test-child-process-spawn-typeerror.js index 791cf02280a3cb..31a1867df945f7 100644 --- a/test/parallel/test-child-process-spawn-typeerror.js +++ b/test/parallel/test-child-process-spawn-typeerror.js @@ -33,7 +33,7 @@ const invalidArgValueError = common.expectsError({ code: 'ERR_INVALID_ARG_VALUE', type: TypeError }, 14); const invalidArgTypeError = - common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError }, 10); + common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError }, 12); assert.throws(function() { const child = spawn(invalidcmd, 'this is not an array'); @@ -76,6 +76,14 @@ assert.throws(function() { spawn(cmd, [], 1); }, invalidArgTypeError); +assert.throws(function() { + spawn(cmd, [], { uid: 2 ** 63 }); +}, invalidArgTypeError); + +assert.throws(function() { + spawn(cmd, [], { gid: 2 ** 63 }); +}, invalidArgTypeError); + // Argument types for combinatorics. const a = []; const o = {}; From 7952acddfe42ac1b1faf62843ae512835dc43fcf Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Wed, 29 Aug 2018 04:45:36 +0300 Subject: [PATCH 2/2] fixup! child_process: fix handling of incorrect uid/gid in spawn --- lib/child_process.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 214f41f2c36dbf..118927d9fa1f83 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -37,7 +37,7 @@ const { ERR_INVALID_OPT_VALUE, ERR_OUT_OF_RANGE } = require('internal/errors').codes; -const { validateString, isUint32 } = require('internal/validators'); +const { validateString, isInt32 } = require('internal/validators'); const child_process = require('internal/child_process'); const { _validateStdio, @@ -425,15 +425,13 @@ function normalizeSpawnArguments(file, args, options) { } // Validate the uid, if present. - if (options.uid != null && - (!Number.isInteger(options.uid) || !isUint32(options.uid))) { - throw new ERR_INVALID_ARG_TYPE('options.uid', 'uint32', options.uid); + if (options.uid != null && !isInt32(options.uid)) { + throw new ERR_INVALID_ARG_TYPE('options.uid', 'int32', options.uid); } // Validate the gid, if present. - if (options.gid != null && - (!Number.isInteger(options.gid) || !isUint32(options.gid))) { - throw new ERR_INVALID_ARG_TYPE('options.gid', 'uint32', options.gid); + if (options.gid != null && !isInt32(options.gid)) { + throw new ERR_INVALID_ARG_TYPE('options.gid', 'int32', options.gid); } // Validate the shell, if present.