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

Avoid yaml.load, and prefer yaml.safe_load #95

Merged
merged 1 commit into from
Nov 23, 2016

Conversation

alex
Copy link
Contributor

@alex alex commented Nov 23, 2016

yaml.load ought to be named yaml.danger_load: it can execute arbitrary code (http://pyyaml.org/wiki/PyYAMLDocumentation#LoadingYAML)

yaml.load ought to be named yaml.danger_load: it can execute arbitrary code (http://pyyaml.org/wiki/PyYAMLDocumentation#LoadingYAML)
@oliverchang
Copy link
Collaborator

We can consider these yaml files to be trusted, but I suppose this doesn't hurt.

@oliverchang oliverchang merged commit 1c413fb into google:master Nov 23, 2016
@alex alex deleted the patch-2 branch November 23, 2016 16:25
@alex
Copy link
Contributor Author

alex commented Nov 23, 2016

Writing some extended words on why I think this is important, even though they are, as you note, trusted in this case:

  1. Users change, some day this might morph into something more self-service, and there's no guarantee anyone will remember in the future.
  2. This trains readers and authors to remember to use the safer API in other contexts
  3. Principle of least privilege: the privilege of executing arbitrary code from a configuration file is not necessary to the authors function

Cheers!

DavidKorczynski pushed a commit that referenced this pull request Jul 9, 2024
Fixes #27

---------

Co-authored-by: Jim Choi <jimchoi@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants