Ruby prevent command injection with open3

Question

in one of the project I am working with, we were using backtip approach to run system commands.

resp = `7z x #{zip_file_path} -p#{password} -o#{output_path}`

which works fine. But since it might lead to command injection vulnerability we are planning to use exec or open3. With open3 we are facing issue in executing system commands. We referred this to resolve command injection.

stdin, stdout, stderr = Open3.popen3("7z", "x", zip_file_path, "-p", password, "-o", output_path)

But this leads to below error

error = stderr.readlines
# ["\n", "\n", "Command Line Error:\n", "Too short switch:\n", "-o\n"]

This works when I include params like this.

stdin, stdout, stderr = Open3.popen3("7z", "x", zip_file_path, "-p#{password}", "-o#{output_path}")

But shouldn't we pass arguments separately to avoid command injection? Or Am I doing anything wrong with first version?

Answer 1

Your second version (the one that works):

stdin, stdout, stderr = Open3.popen3("7z", "x", zip_file_path, "-p#{password}", "-o#{output_path}")

is perfectly safe. There's no shell involved so there's nothing that will interpret password and output_path other than 7z itself.

---

Unfortunately 7z has a strange way of parsing switches. Normally you'd expect to pass arguments to switches by saying -p xyz and -pxyz would be a short form of -p -x -y -z. When you say:

Open3.popen3("7z", "x", zip_file_path, "-p", password, "-o", output_path)

that's like saying this in the shell:

7z x zip_file_path -p password -o output_path

but 7z doesn't like those spaces, it wants:

7z x zip_file_path -ppassword -ooutput_path

so you're stuck doing a bit of string interpolation. Even so, there's no shell involved with Open3 so no injection.