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

[requirements] bumping pyyaml to 5.1 #3839

Merged
merged 3 commits into from May 6, 2019
Merged

[requirements] bumping pyyaml to 5.1 #3839

merged 3 commits into from May 6, 2019

Conversation

truthbk
Copy link
Member

@truthbk truthbk commented Apr 30, 2019

What does this PR do?

Bumps PyYaml to the latest 5.1.

Motivation

Enable new full_load() option for customers perhaps wanting to use that after load_all() was patched for security reasons.

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

@hydrosquall
Copy link
Member

Chiming in b/c I've been waiting on updating pyyaml in my own python package for a little while and have been following this issue closely since the 4.x release last summer, - since DD enforces an API boundary in ddyaml.py which monkey-patches yaml.load to be safe, it looks like the upgrade doesn't require any internal API rewrite.

However, there are some possible internal small internal breakages that will be patched in 5.2, as indicated in this release note: yaml/pyyaml#265

@truthbk truthbk merged commit b318272 into master May 6, 2019
@truthbk truthbk deleted the jaime/pyyaml_bump branch May 6, 2019 13:20
@truthbk
Copy link
Member Author

truthbk commented May 6, 2019

Note: Further context on why the monkey patch has been kept in place follows. The pyyaml authors go on to say the following about the 5.1 release yaml/pyyaml#257):

We still recommend that people choose SafeLoader for untrusted data, but
aribitrary code execution will no longer be possible using yaml.load() with
the default loader (FullLoader). FullLoader will instantiate objects of classes
that you have imported. Since object instantiation runs the class's constructor code, that may be exploitable.

Because the FullLoader still presents some exploitable vector we will wait for 6.0, when yaml.load() will raise an exception if no loader is specified in the calling code, to remove the monkey patch.

remeh added a commit that referenced this pull request May 14, 2019
remeh pushed a commit that referenced this pull request May 14, 2019
* [requirements] bumping pyyaml to 5.1

* [gemfile] pinning parallel to ruby 2.2 compatible version

* [gemfile] address alphabetical cop
remeh added a commit that referenced this pull request May 22, 2019
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.

None yet

3 participants