Reputation: 1215
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?
Upvotes: 11
Views: 6352
Reputation: 681
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:
Upvotes: 12