Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potentially unsafe subprocess execution (shell=True) #182

Open
mikewhiteman opened this issue Jul 22, 2020 · 2 comments
Open

Potentially unsafe subprocess execution (shell=True) #182

mikewhiteman opened this issue Jul 22, 2020 · 2 comments

Comments

@mikewhiteman
Copy link

Hello 👋 - Any particular reason that subprocess needs to be run with Shell=True?

An example:

https://github.com/OrkoHunter/pep8speaks/blob/837643bb95c18a5364cd0539b0f8edaeb3813a76/pep8speaks/helpers.py#L234

Shell=True is a more dangerous way of invoking subprocess that exposes you to potential command injection attacks. If you're not using any shell specific features (e.g. |), changing it to shell=False is a pretty easy fix.

@OrkoHunter
Copy link
Collaborator

Related #28

@sarnold
Copy link

sarnold commented Oct 9, 2021

I think the answer is basically split the cmd string: https://github.com/OrkoHunter/pep8speaks/blob/30af5bf0de1a2bfdf35b0a76bf63c70486c12c69/pep8speaks/helpers.py#L231 and pass cmd as a list to Popen , then you can set shell=False and you're done. Lastly (and I just learned this recently) running bandit on the source code will find this kind of thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants