-
Notifications
You must be signed in to change notification settings - Fork 442
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
added models for python-sh #518
base: main
Are you sure you want to change the base?
Conversation
fc50a7f
to
0ec90d9
Compare
A few notes belowThe python-sh library is included in the virtual enviroments site-packages as a single module (python file) To ensure pysa picks the module, you should create an Example
This affects the way I import sh in the |
We're looking into the bug with Pyre not picking up |
@0xedward has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
okay |
Hey @esiebomaj! @shannonzhu landed a fix for this in aa1034d. Would you mind updating your local pyre config to match the following and let us know if Pyre is able to pick up {
"taint_models_path": "../../stubs",
"search_path": [
"../../stubs",
{
"site-package": "sh",
"is_toplevel_module": true
},
{
"site-package": "flask"
},
],
"source_directories": [
"."
]
} |
Pre-submission checklist
pip install -r requirements-dev.txt && pre-commit install
pre-commit run
Summary
This PR adds pysa models for python-sh
Python-sh is a full-fledged replacement for subprocess in python. This might allow arbitrary command execution if user-controlled data is allowed to flow to some of this module's functions
therefore It is important to have pysa models for this library in other to prevent this occurrence
Test Plan
For testing, I have included some functions in
documentation\deliberately_vulnerable_flask_app
which uses python-sh and may potentially cause RCEcd documentation\deliberately_vulnerable_flask_app
run
pyre analyze --no-verify
to see the vulnerabilities pysa has detected in the app.pyRelated Issue: #MLH-Fellowship-issue-57