Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix command injection vulnerability
@ronomon/opened was vulnerable to a command injection vulnerability
that would allow a remote attacker to execute commands on the system if
the library was used with untrusted input.

The root cause of the problem was line 87 in index.js which took
potential untrusted input as part of a string executed as a command
by `child_process.exec()`. While the arguments were escaped by
@ronomon/opened, an attacker could still bypass this sanitization
because `child_process.exec()` will also interpret the string as a
shell command.

This fix moves to `execFile` to spawn the binary with separate
arguments that will not also be interpreted as shell commands.

Thanks to Fábio Freitas, a security analyst at Checkmarx's CxSCA group,
for discovering and disclosing the vulnerability, providing clear steps
to reproduce and suggestions for mitigation.
  • Loading branch information
jorangreef committed Apr 14, 2021
1 parent 33c2dfb commit 7effe01
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
10 changes: 3 additions & 7 deletions index.js
Expand Up @@ -79,17 +79,13 @@ Unix.files = function(paths, end) {
var files = {};
var queue = new Queue(1); // Concurrency yields no improvement with lsof.
queue.onData = function(paths, end) {
var escapedPaths = paths.map(
function(path) {
return '"' + path.replace(/"/g, '\\"') + '"';
}
);
var command = 'lsof -F n -- ' + escapedPaths.join(' ');
var command = 'lsof';
var args = ['-F', 'n', '--'].concat(paths);
var options = {
encoding: 'utf-8',
maxBuffer: 2 * 1024 * 1024
};
Node.child.exec(command, options,
Node.child.execFile(command, args, options,
function(error, stdout, stderr) {
// lsof returns an error and a status code of 1 if a file is not open:
if (error && error.code === 1 && stderr.length === 0) error = undefined;
Expand Down
15 changes: 15 additions & 0 deletions test-injection.js
@@ -0,0 +1,15 @@
const assert = require('assert');
const fs = require('fs');

const Opened = require('./index.js');

const paths = ['$(touch command_line_injection)'];

Opened.files(paths,
function(error, hashTable) {
assert(!!error);
assert(fs.existsSync('command_line_injection') === false);
console.log('PASS');
}
);

0 comments on commit 7effe01

Please sign in to comment.