child_process: Check stderr before accessing it

robertchiras · Fishrock123 · commit 765de1ae1143 · 2016-07-19T13:27:30.000+02:00
If something bad happens in spawnSync, stderr might be null. Therefore, we have to check it before using it, so we won't mask the actual exception. PR-URL: #6877 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
diff --git a/lib/child_process.js b/lib/child_process.js
@@ -494,7 +494,7 @@ function execFileSync(/*command, args, options*/) {
 
   var ret = spawnSync(opts.file, opts.args.slice(1), opts.options);
 
-  if (inheritStderr)
+  if (inheritStderr && ret.stderr)
     process.stderr.write(ret.stderr);
 
   var err = checkExecSyncError(ret);
@@ -514,7 +514,7 @@ function execSync(command /*, options*/) {
   var ret = spawnSync(opts.file, opts.options);
   ret.cmd = command;
 
-  if (inheritStderr)
+  if (inheritStderr && ret.stderr)
     process.stderr.write(ret.stderr);
 
   var err = checkExecSyncError(ret);
diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js
@@ -12,6 +12,18 @@ var start = Date.now();
 var err;
 var caught = false;
 
+// Verify that stderr is not accessed when a bad shell is used
+assert.throws(
+  function() { execSync('exit -1', {shell: 'bad_shell'}); },
+  /spawnSync bad_shell ENOENT/,
+  'execSync did not throw the expected exception!'
+);
+assert.throws(
+  function() { execFileSync('exit -1', {shell: 'bad_shell'}); },
+  /spawnSync bad_shell ENOENT/,
+  'execFileSync did not throw the expected exception!'
+);
+
 try {
   var cmd = `"${process.execPath}" -e "setTimeout(function(){}, ${SLEEP});"`;
   var ret = execSync(cmd, {timeout: TIMER});
