Skip to content

Commit

Permalink
fix(GUI): surround paths in double quotes before elevating (#779)
Browse files Browse the repository at this point in the history
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: #773
Change-Type: patch
Changelog-Entry: Prevent escaping issues during elevation by surrounding paths in double quotes.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>
  • Loading branch information
Juan Cruz Viotti authored Oct 27, 2016
1 parent b6817cf commit fdbb767
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 38 deletions.
32 changes: 1 addition & 31 deletions lib/src/child-writer/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@

'use strict';

const _ = require('lodash');
const os = require('os');

/**
* @summary Get the explicit boolean form of an argument
* @function
Expand Down Expand Up @@ -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,
Expand All @@ -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, '\\ ');
});
};
17 changes: 10 additions & 7 deletions lib/src/child-writer/writer-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) => {
Expand Down

0 comments on commit fdbb767

Please sign in to comment.