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

PyYAML is vulnerable #3828

Closed
syabro opened this issue Jan 5, 2019 · 14 comments
Closed

PyYAML is vulnerable #3828

syabro opened this issue Jan 5, 2019 · 14 comments
Labels
blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. feature-request A feature should be added or improved.

Comments

@syabro
Copy link

syabro commented Jan 5, 2019

Current version is

PyYAML>=3.10,<=3.13

Should be updated to 4.1b4 or higher

https://nvd.nist.gov/vuln/detail/CVE-2017-18342

@stealthycoin
Copy link
Contributor

We do not use yaml.load with the default unsafe loader which is what the CVE is about.

@stealthycoin stealthycoin added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jan 7, 2019
@syabro
Copy link
Author

syabro commented Jan 7, 2019

@stealthycoin but we use and having aws-cli in requirements blocks update in our project :(

@no-response no-response bot removed the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jan 7, 2019
@gchiesa
Copy link

gchiesa commented Jan 8, 2019

Are you planning to update anyway the pinning the the not vulnerable version of PyYAML? It would solve also for us the update of dependencies and security checks of our project.

@maciejstromich
Copy link

@stealthycoin we're waiting for an update as well. It's blocking our security checks as well.

@maciejstromich
Copy link

BTW one way to mitigate is to pin PyYAML version to 4.2b1 before installing awscli.
pip complains about a newer version but it shouldn't be an issue (just made few simple tests to see if my use cases work)

@kyleknap
Copy link
Contributor

kyleknap commented Jan 8, 2019

So another thing to keep in mind is that version of 4.2b4 is only a pre-release. According to PyYaml's release history, the latest stable release is considered 1.13, which does not default to the safe loader, as the stable release of4.1 was removed from PyPI. In general, the CLI should only be taking dependencies on stable versions of a library. Looking over the PyYaml GitHub repository, there is an issue tracking the release of a 4.2: yaml/pyyaml#193, which would be considered stable.

Given we do not load with the unsafe loader, I think we will have to wait until 4.2 is released, in which we can then proceed in looking to upgrade. Otherwise, the only other option would be to switch out our dependencies to use the ruamel.yaml library. We use that library for v2 of the CLI, but it may be a difficult change to make without breaking any existing functionality for v1 of the CLI.

@kyleknap kyleknap added blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. feature-request A feature should be added or improved. labels Jan 8, 2019
@maciejstromich
Copy link

@kyleknap awesome. thanks for the information.

Personally I'm good with the mitigation path outlined in my previous comment so my PR won't get blocked by our security scans.

@koleror
Copy link

koleror commented Jan 9, 2019

BTW one way to mitigate is to pin PyYAML version to 4.2b1 before installing awscli.
pip complains about a newer version but it shouldn't be an issue (just made few simple tests to see if my use cases work)

The problem is it doesn't go through pip-compile, which has the purpose of avoiding exactly this...

@ViktorHaag
Copy link

The latest stable release of PyYAML (5.1) after 3.13 has dropped support for Python2.6, which I believe is a problem for this project (see #3660), and will likely delay moving forward on this dependency. 8(

@jessedoyle
Copy link

jessedoyle commented Apr 18, 2019

Just a question - what is the concern (if any) about relaxing the the restriction on pyyaml to only >=3.10?

This would allow host applications to specify a non-vulnerable version of pyyaml if necessary.

I'm not very familiar with how pip resolves versions, but I'm assuming it'll install the latest possible version (introducing incompatibilities)?

EDIT: It looks like a similar approach is already being considered in #4015. I should have dug a bit deeper!

chrisgemignani added a commit to juiceinc/devlandia that referenced this issue Apr 24, 2019
Fixes security issues with Jinja2, urllib3, pyyaml. Raises an error when installing pyyaml==5.1 due to pinned constraints in docker-compose and aws-cli.

AWS-cli has issues w pyyaml 5 dropping support for python 2.6
aws/aws-cli#3828

Docker compose is still waiting on some issues w unicode support in pyyaml 5.1, but this doesn't affect us.
docker/compose#6623
@tpanza
Copy link

tpanza commented May 20, 2019

May I ask why is this still blocked?

It looks like #4015 has a provision for allowing Python 2.6 users to stay with a compatible version of PyYAML (3.13), while allowing Python 2.7+ users to finally move to a newer version of PyYAML (5.1).

Can the solution in #4015 be merged, and then track the issue of Python 2.6 users still having a vulnerable PyYAML dependency in a new issue?

bgmilne added a commit to bgmilne/aws-cli that referenced this issue May 22, 2019
Most current linux distributions ship newer versions than the current
dependencies permit of:
* colorama (only used/required on Windows)
* rsa (e.g. 4.0, which doesn't support python 2.6)
* PyYAML (e.g. 5.1 which doesn't support python 2.6)

This change makes the legacy dependencies only take effect on those
legacy platforms that can't use the modern versions.

Fixes aws#3660
Fixes aws#3828
@SeppPenner
Copy link

The currently used [email protected] contains a Arbitrary Code Execution issue. Check https://app.snyk.io/vuln/SNYK-PYTHON-PYYAML-42159 and https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-18342 for more information. (Just as reference for the duplicate issue I added here: #4193)

@glyph
Copy link

glyph commented Aug 21, 2019

Undoubtedly the real issue here is the lack of subtlety and manageability of vulnerability descriptions, but nevertheless, the pin here is creating lots of spurious github security advisories and the real solution to the noise is to just allow for upgrades.

(Edit: this was a mistaken belief generated by a CI workaround that was watching for this issue to be closed, not watching the pin itself...)

@dstufft
Copy link
Contributor

dstufft commented Aug 21, 2019

Closing this as we are currently pinning to <5.2 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.