Sanitized input for subprocess with shell=True in python

Question

I have python script, which has code.

...
...
p = subprocess.Popen(cmd,
                     stdout=subprocess.PIPE,
                     stderr=subprocess.PIPE,
                     shell=True)
output, error = p.communicate()
...
...

When I run bandit it gives error.

>> Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue.
   Severity: High   Confidence: High
   Location: mypackage/myfile.py:123
123                                          stderr=subprocess.PIPE,
124                                          shell=True)
125                     output, error = p.communicate()

Then I do some google, and found that, I have to sanitized my input and with shlex.split and shlex.quote I can sanitize it.

I changed my code to.

...
...
p = subprocess.Popen(shlex.split(shlex.quote(cmd)),
                     stdout=subprocess.PIPE,
                     stderr=subprocess.PIPE,
                     shell=True)
output, error = p.communicate()
...
...

But still I get same error, is there any way to remove this error when run bandit -r mypackage/myfile.py

Answer 1

So, user enter command which he want to run

If the user already may run any command including bash then the bandit 's warning about shell=True is not applicable.

The warning would make sense if the user were allowed only to choose some parameters for a fixed command e.g., a search query for a grep command:

rc = call(['grep', '-e', query, path])

whatever user-specified query is; it won't make it to run some other command (only grep is run).

Compare it with shell=True:

rc = call("grep -e '%s' '%s'" % (query, path), shell=True) #XXX don't do it

A user could pass query = "a' /dev/null; rm -rf '" that would produce grep -e 'a' /dev/null; rm -rf '' 'path' command.

shell=True allows a user to run an arbitrary command in this case even if it is not intended. It is called a shell injection.

You could call pipes.quote(query), to avoid naive attacks but it may fail in the general case that is why shell=True should be avoided if the input is not from a trusted source.