child_process spawn in node.js security / escaping

Question

In Node, I'm using a module (GM) and noticed that it uses spawn from the child_process module to pass arguments to GraphicMagick's convert executable.

I'm passing user-submitted information to GM. Is there a security concern that the user could do some sort of injection attack using a pipe (or other command line trickery)? Or does spawn protect against that? If not, is there a best practice for escaping user submitted values in this case?

Stanfield

Answer 1

We recently published a blog post on avoiding command injection vulnerabilities in node.js. It explains a bit about how spawn prevents this.

If gm was using child_process.exec there would be a greater chance of injection. This is because child_process.exec executes the commands under a subshell and not directly, letting shell meta characters like backticks, $(), ;, &&, || etc to be used nefariously.

The resulting system call looks like this with .exec() for a simple ls -l that might take user input.

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

Since gm uses spawn the resulting system call would look something like this.

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

As gm would be the first argument to execve. This means that a user cannot run subcommands in the shell using pipes and other command line trickery, because in our example /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 parametrized vs string-based SQL queries, if you are familiar with that.

This does however come with a caveat: using spawn is not always a safe thing. User provided arguments could still potentially have a bad outcome, maybe not command injection but something else. Check with the behavior of gm and the arguments that you are pass in user provided input into and think about how the user might be able to abuse that argument.

So, here’s the generic 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.

Hayne

What about using several commands? For example "cd" + dir + "; ./" + fileToExecute. I guess you wouldn't be able with the way you described. Am I wrong? – Broadsword