From fdbb7673a6eced691dd9e412b407dfc31a32005a Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Thu, 27 Oct 2016 11:39:14 -0400 Subject: [PATCH] fix(GUI): surround paths in double quotes before elevating (#779) This PR makes sure every command line argument that represents an absolute path is surrounded by double quotes, to avoid any potential escaping issue. This simplifies a lot the various special character escaping routines we had in place, since we now only have to make sure double quotes inside the paths are escaped. Fixes: https://github.com/resin-io/etcher/issues/773 Change-Type: patch Changelog-Entry: Prevent escaping issues during elevation by surrounding paths in double quotes. Signed-off-by: Juan Cruz Viotti --- lib/src/child-writer/utils.js | 32 +--------------------------- lib/src/child-writer/writer-proxy.js | 17 +++++++++------ 2 files changed, 11 insertions(+), 38 deletions(-) diff --git a/lib/src/child-writer/utils.js b/lib/src/child-writer/utils.js index 72086d08cd..76b7785dd1 100644 --- a/lib/src/child-writer/utils.js +++ b/lib/src/child-writer/utils.js @@ -16,9 +16,6 @@ 'use strict'; -const _ = require('lodash'); -const os = require('os'); - /** * @summary Get the explicit boolean form of an argument * @function @@ -72,17 +69,7 @@ exports.getBooleanArgumentForm = (argumentName, value) => { exports.getCLIWriterArguments = (options) => { const argv = [ options.entryPoint, - _.attempt(() => { - if (os.platform() === 'win32') { - return options.image; - } - - // Parenthesis and quotes need to be manually escaped, otherwise - // bash will complain about syntax errors when passing this - // string as an argument to the writer proxy script. - return options.image.replace(/([\(\)'"])/g, '\\$1'); - - }), + options.image, '--robot', '--drive', options.device, @@ -98,20 +85,3 @@ exports.getCLIWriterArguments = (options) => { return argv; }; - -/** - * @summary Escape white spaces from arguments - * @function - * @public - * - * @param {String[]} argv - argv - * @returns {String[]} escaped arguments - * - * @example - * const escapedArguments = utils.escapeWhiteSpacesFromArguments(process.argv); - */ -exports.escapeWhiteSpacesFromArguments = (argv) => { - return _.map(argv, (argument) => { - return argument.replace(/\s/g, '\\ '); - }); -}; diff --git a/lib/src/child-writer/writer-proxy.js b/lib/src/child-writer/writer-proxy.js index ac1b6214c5..961aaf5c6b 100644 --- a/lib/src/child-writer/writer-proxy.js +++ b/lib/src/child-writer/writer-proxy.js @@ -26,7 +26,6 @@ const path = require('path'); const sudoPrompt = Bluebird.promisifyAll(require('sudo-prompt')); const EXIT_CODES = require('../exit-codes'); const packageJSON = require('../../../package.json'); -const utils = require('./utils'); // This script is in charge of spawning the writer process and // ensuring it has the necessary privileges. It might look a bit @@ -75,7 +74,7 @@ return isElevated().then((elevated) => { }); } - const command = _.attempt(() => { + const commandArguments = _.attempt(() => { const commandPrefix = [ // Some elevation tools, like `pkexec` or `kdesudo`, don't @@ -104,15 +103,19 @@ return isElevated().then((elevated) => { return commandPrefix .concat([ process.env.APPIMAGE ]) - .concat(utils.escapeWhiteSpacesFromArguments(translatedArguments)) - .join(' '); + .concat(translatedArguments); } - return commandPrefix.concat( - utils.escapeWhiteSpacesFromArguments(process.argv) - ).join(' '); + return commandPrefix.concat(process.argv); }); + const command = _.join(_.map(commandArguments, (argument) => { + return `"${argument.replace(/(")/g, '\\$1')}"`; + }), ' '); + + // For debugging purposes + console.log(`Running: ${command}`); + return sudoPrompt.execAsync(command, { name: packageJSON.displayName }).then((stdout, stderr) => {