Merge pull request #1 from Mik317/master

[Fix] Patched RCE arising inside the "launchpad" library
418sec · Jul 3, 2020 · de5aca1 · de5aca1
2 parents a2b5755 + a3ff180
commit de5aca1
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 7 deletions.
diff --git a/lib/local/instance.js b/lib/local/instance.js
@@ -1,12 +1,20 @@
 var path = require('path');
 var spawn = require("child_process").spawn;
 var exec = require("child_process").exec;
+var execFile = require("child_process").execFile;
 var EventEmitter = require('events').EventEmitter;
 var debug = require('debug')('launchpad:local:instance');
 var rimraf = require('rimraf');
 
+var safe = function (str) {
+   // Avoid quotes makes impossible escape the `multi command` scenario
+   return str.replace(/['"]+/g, '');
+}
+
 var getProcessId = function (name, callback) {
 
+  name = safe(name);
+
   var commands = {
     darwin: "ps -clx | grep '" + name + "$' | awk '{print $2}' | head -1",
     linux: "ps -ax | grep '" + name + "$' | awk '{print $2}' | head -1",
@@ -90,13 +98,16 @@ Instance.prototype.stop = function (callback) {
     } catch (error) {}
   } else {
     if (this.options.command.indexOf('open') === 0) {
-      command = 'osascript -e \'tell application "' + self.options.process + '" to quit\'';
+      command = 'osascript -e \'tell application "' + safe(self.options.process) + '" to quit\'';
       debug('Executing shutdown AppleScript', command);
-      exec(command);
+      command = command.split(' ');
+      execFile(command[0], command.slice(1));
     } else if (process.platform === 'win32') {
-      command = 'taskkill /IM ' + (this.options.imageName || path.basename(this.cmd));
+      //Adding `"` wasn't safe/functional on Win systems
+      command = 'taskkill /IM ' + (this.options.imageName || path.basename(this.cmd)); 
       debug('Executing shutdown taskkil', command);
-      exec(command).once('exit', function(data) {
+      command = command.split(' ');
+      execFile(command[0], command.slice(1)).once('exit', function(data) {
         self.emit('stop', data);
       });
     } else {

diff --git a/lib/local/version.js b/lib/local/version.js
@@ -1,11 +1,22 @@
 var fs = require('fs');
 var exec = require('child_process').exec;
+var execFile = require('child_process').execFile;
 var Q = require('q');
 var path = require('path');
 var plist = require('plist');
 var utils = require('./utils');
 var debug = require('debug')('launchpad:local:version');
 
+// Validate paths supplied by the user in order to avoid "arbitrary command execution"
+var validPath = function (filename){
+  var filter = /[`!@#$%^&*()_+\-=\[\]{};'"|,<>?~]/;
+  if (filter.test(filename)){
+    console.log('\nInvalid characters inside the path to the browser\n');
+    return
+  }
+  return filename;
+}
+
 module.exports = function(browser) {
   if (!browser || !browser.path) {
     return Q(null);
@@ -18,7 +29,7 @@ module.exports = function(browser) {
 
     debug('Retrieving version for windows executable', command);
     // Can't use Q.nfcall here unfortunately because of non 0 exit code
-    exec(command, function(error, stdout) {
+    execFile(command.split(' ')[0], command.split(' ').slice(1), function(error, stdout) {
       var regex = /ProductVersion:\s*(.*)/;
       // ShowVer.exe returns a non zero status code even if it works
       if (typeof stdout === 'string' && regex.test(stdout)) {
@@ -47,8 +58,8 @@ module.exports = function(browser) {
   }
 
   // Try executing <browser> --version (everything else)
-  return Q.nfcall(exec, browser.path + ' --version').then(function(stdout) {
-    debug('Ran ' + browser.path + ' --version', stdout);
+  return Q.nfcall(exec, validPath(browser.path) + ' --version').then(function(stdout) {
+    debug('Ran ' + validPath(browser.path) + ' --version', stdout);
     var version = utils.getStdout(stdout);
     if (version) {
       browser.version = version;