[Python-ideas] shutil.run (Was: shutil.runret and shutil.runout)

[anatoly techtonik]

About security.

On Tue, May 22, 2012 at 11:30 PM, Mike Meyer <mwm at mired.org> wrote:
> On Tue, 22 May 2012 18:39:16 +0300
> anatoly techtonik <techtonik at gmail.com> wrote:
>
>> Therefore, inspired by Fabric API, I've finally found the solution -
>> shutil.run() function:
>> https://bitbucket.org/techtonik/shutil-run/src
>>
>> run(command, combine_stderr=True):
>>
>>     Run command through a system shell, return output string with
>>     additional properties:
>>
>>         output.succeeded    - result of the operation True/False
>>         output.return_code  - specific return code
>>         output.stderr       - stderr contents if combine_stderr=False
>>
>>      `combine_stderr` if set, makes stderr merged into output string,
>>      otherwise it will be available  as `output.stderr` attribute.
> [...]
>> That's the most intuitive way I found so far. Objective advantages:
>>
>> 1. Better than
>>        subprocess.call(cmd, shell=true)
>>        subprocess.check_call(cmd, shell=true)
>>        subprocess.check_output(cmd, shell=True)
>>      because it is just
>>        shutil.run(cmd)
>>      i.e. short, simple and _easy to remember_
>
> -2
>
> Unless there's some way to turn off shell processing (better yet, have
> no shell processing be the default, and require that it be turned on),
> it can't be used securely with tainted strings, so it should *not* be
> used with tainted strings, which means it's pretty much useless in any
> environment where security matters. With everything being networked,
> there may no longer be any such environments.

What does this "shell processing" involve to understand what to turn off?
Why there is no way to turn off "shell processing"?
What's the primary reason that it is impossible to be turned off?

>> 3. shutil.run() is predictable and consistent - its arguments are not
>> dependent on each other, their combination doesn't change the function
>> behavior over and over requiring you iterate over the documentation
>> and warnings again and again
>
> As proposed, it certainly provides a predictable and consistent
> vulnerability to code injection attacks.

suprocess.* with shell=True provides the same entrypoint for injection
attacks, and security through obscurity doesn't help here. People
still use shell=True, because that's sometimes the only way to execute
external utilities properly. Even my synapses were silent when I
reviewed and used shell=True for Rietveld upload script and Spyder
IDE.

What will help is a better simple explanation in a prominent place,
with an example that people can really remember instead of frightening
them with warnings. People will ignore warning eventually, and after
endless experiments will subprocess.* params mess will just leave
shell=True because it works (I did so).

No sane web developer will use subprocess calls on server side at all.
Regardless of shell=True or not. For example, how can I be sure that
Graphviz is save from exploit through malicious input? No sane
developer will run shell script on a web side either. For those who
still want - there will be this simple explanation right on the
shutil.run() page - with link to proper vulnerability analysis instead
of uncertainty inducting warning.

shutil.run() is aimed for local operations.

>> 4. shutil.run() is the correct next level API over subprocess base
>> level. subprocess executes external process - that is its role, but
>> automatic ability to execute external process inside another external
>> process (shell) looks like a hack to me. Practical, but still a hack.
>
> It's only correct if you are in an environment where you don't care
> about security. If you care about security, you can't use it. If we're
> going to add yet another system() replacement, let's at least try and
> make it secure.

I am all ears how to make shutil.run() more secure. Right now I must
confess that I don't even realize.how serious is this problems, so if
anyone can came up with a real-world example with explanation of
security concern that could be copied "as-is" into documentation, it
will surely be appreciated not only by me.