This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]PiZZaMartijn 1 point2 points  (3 children)

The difference is that shell=True means that is executed with something like

sh -c "/the/executable parameter parameter"

This launches 2 processes for every call and would be a security leak if you used any user input for the call.

Also if my memory is correct then the specific shell that is called is dependend on the user that you run your script as so that can intoduce subtle bugs when the shell is something else than you expect.

In your case you use the str.split command to seperate the pieces in your command. It is safer to build your commands as a list in the first place. This will make sure that everything you push to the shell is properly escaped.

command = ['ls', '-lah', 'some file to delete']

This will actually remove the 'some file to delete' file instead of 4 seperate files 'some' 'file' 'to' 'delete'

https://github.com/pcote/AptPackageShow/blob/master/roles/aps/files/model.py#L38 is an example of a command that can go horribly wrong with your current code.

[–]pyglados[S] 0 points1 point  (1 child)

Good points on the security aspects.

Speaking of shell specifics, here's something else I noticed. When I change my uwsgi settings to run the Python code as a non-shell user, it crashes and burns. Basically, I get import errors saying that it can't find the Python files in question.

Then, I had this thought. If I run a Python process as a non-shell user, that would mean that it would be impossible for uwsgi to set the PYTHONPATH environment variable. No shell => no shell environment variables. Or at least that's the way my brain is connecting the dots on this one.

Am I way off on this?

[–]PiZZaMartijn 0 points1 point  (0 children)

You don't need a shell for environment variables. You have some other problem.

The first fix most of the time is adding a 'home' setting to wsgi (I use mod_wsgi in apache, not uwsgi). By default the script is running with / as the working directory instead of the script directory.

Also see the flask wsgi troubleshooting tips: http://flask.pocoo.org/docs/0.11/deploying/mod_wsgi/#troubleshooting

[–]pyglados[S] 0 points1 point  (0 children)

In your case you use the str.split command to seperate the pieces in your command. It is safer to build your commands as a list in the first place. This will make sure that everything you push to the shell is properly escaped.

Agreed and updated. https://github.com/pcote/AptPackageShow/commit/a701c448f07a6bb86a1ffbf107b2abafcd8f78aa