Skip to content

Instantly share code, notes, and snippets.

@evilpacket
Created April 3, 2019 21:26
Show Gist options
  • Star 6 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save evilpacket/5a9655c752982faf7c4ec6450c1cbf1b to your computer and use it in GitHub Desktop.
Save evilpacket/5a9655c752982faf7c4ec6450c1cbf1b to your computer and use it in GitHub Desktop.
date slug tags title author type
2014-08-19 17:04:34 GMT
Avoid-Command-Injection-Node.js
security, node.js, injection
Avoiding Command Injection in Node.js
Adam Baldwin
text

In this post we are going to learn about the proper way to call a system command using node.js to avoid a common security flaw, command injection.

A call that we often see used, due to it's symplicity is child_process.exec. It's got a simple pattern; pass in a command string and it calls you back with an error or the command results.

Here is a very typical way you would call a system command with child_process.exec.

child_process.exec('ls', function (err, data) {
    console.log(data);
});

What happens though when you need to start getting user input for arguments into your command? The obvious solution is to take the user input and build your command out using string concatenation. But here's something I've learned over the years: When you use string concatentation to send data from one system to another you're probably going to have a bad day.

var path = "user input";
child_process.exec('ls -l ' + path, function (err, data) {
    console.log(data);
});

Why is string concatenation a problem?

Well, because under the hood, child_process.exec makes a call to execute '/bin/sh' rather than the target program. The command that was sent just gets passed along as a shell command in the newly spawned '/bin/sh' process. child_process.exec has a misleading name - it's a bash interpreter, not a program launcher. And that means that all shell metacharacters can have devestating effects if the command is including user input.

[pid 25170] execve("/bin/sh", ["/bin/sh", "-c", "ls -l user input"], [/* 16 vars */] 

For example, an attacker could use a ; to end the statement and start another one, they could use backticks or $() to run a subcommand. Lots of potential for abuse.

So how do we do this the right way?

execFile / spawn

Calls like spawn and execFile take additional command arguments as an array, are not executed under a shell environment, and do not manipulate the originally intended command to run.

Let's modify our example to use execFile and spawn and see how the system calls differ, and why it isn't vulnerable to command injection.

child_process.execFile

var child_process = require('child_process');

var path = "."
child_process.execFile('/bin/ls', ['-l', path], function (err, result) {
    console.log(result)
});

System call that is run

[pid 25565] execve("/bin/ls", ["/bin/ls", "-l", "."], [/* 16 vars */]

child_process.spawn Similar example using spawn instead.

var child_process = require('child_process');

var path = "."
var ls = child_process.spawn('/bin/ls', ['-l', path])
ls.stdout.on('data', function (data) {
    console.log(data.toString());
});

System call that is run

[pid 26883] execve("/bin/ls", ["/bin/ls", "-l", "."], [/* 16 vars */

When using spawn or execFile, our target program is the first argument to execve. This means that a user cannot run subcommands in the shell, because /bin/ls has no idea what to do with backticks or pipes or ;. It's /bin/bash that is going to be interpreting those commands. It's similar to using parameterized vs string-based SQL queries, if you are familiar with that.

This does however come with a caveat: using spawn or execFile is not always a safe thing. For example, running /bin/find with spawn or execFile and passing user input in directly could still lead to complete system takeover. The find command has options that allow for arbitrary file read/write.

So, here's the collective guidance for running system commands from node.js:

  • Avoid using child_process.exec, and never use it if the command contains any input that changes based on user input.
  • Try to avoid letting users pass in options to commands if possible. Typically values are okay when using spawn or execfile, but selecting options via a user controlled string is a bad idea.
  • If you must allow for user controlled options, look at the options for the command extensively, determine which options are safe, and whitelist only those options.

Interested in avoiding other common vulnerabilities in your node.js applications? Consider coming to one of our node.js security training events or contact us to find out more about onsite training for your entire team!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment