Pyyaml: CVE-2017-18342 status

[Jwomers]

Hi, I am maintaining the safety database at PyUp, and I want to get clarity on which version of PyYAML fixes CVE-2017-18342? I know 4.1 was released and then retracted from PyPi.

Does 3.13 fix CVE-2017-18342?

Thanks

[Kobold]

I haven't read the code, but it seems like 3.13 does not fix the CVE. The CVE was regarding the default-unsafe load method, and the change that fixed it (PR #74) was only merged in 4.1 onward, and has since been reverted from the main branch that 4.2 will be cut from.

Issue #193 has the most clarity on the release plan, but it's unclear what the timeline for having #74 (that is, a fix for the CVE) re-merged is. It might be a while before there's a fix for the CVE.

Thanks again for the work you do with PyUp @Jwomers — I would say this is a tricky situation for a safety database. The CVE is not a new thing, it merely got unsafe defaults that have been present since 2006 flagged. Users of PyYAML need to know that they're using a potentially dangerous package, but at the same time, this isn't shocking to anyone who has some familiarity with PyYAML and its sharp edges.

[ingydotnet]

@Kobold's assessment is spot on.

3.13 only addresses PyYAML working with Python 3.7.

The next release (almost certainly versioned as 4.2) has high hopes of including a fix for this issue.

[ljvmiranda921]

Hi, thanks for the pyyaml project. It has been very helpful!
Any updates on this bug? Thank you very much for your time and effort!

[hashar]

The summary from my analysis on our downstream bug https://phabricator.wikimedia.org/T214560#4905113

pyyaml.load allows execution of code, one would always want to use pyyaml.safe_load instead. The proposal in #74 is to change the semantic of load to make it safe by default and to introduce a new danger_load when one knows code should be executed.The table is:

| Method | Current | PR #74
|--|--|--
| load | unsafe | safe
| safe_load | safe | safe
| danger_load | N/A | unsafe

That has been included in 4.1 (which got unreleased), 4.2b1 and 4.2b2. It has been reverted in branches 4.2 and 4.3.

There is a safey-api branch which might be related with commit 3dc3f5f0fc41db52e894f65cc4eec9470ffdf725 . And I guess reimplement the feature in a different way. The only release that has this change is 4.2b1, it is not in 4.2b1 nor in 4.2/4.3 branches.

My conclusion is the CVE complains that load() is unsafe (since people should almost always use safe_load(). The idea behind #74 is to change load() to be safe and add a new method danger_load() for the case when ones wants to allow code execution.

I think the CVE can be dismissed, but at least it raises awareness that code relying on pyyaml should be audited for usage of load() / dump() and those should be converted to safe_load() / safe_dump.

To make it nicer to users that do not pay attention to the possible code execution when using load(), a major release of pyyaml should probably introduce the breaking change of making load() to be safe which would solve the CVE.

[hashar]

Also from Debian https://security-tracker.debian.org/tracker/CVE-2017-18342

This is a well-known design deficiency in pyyaml, various CVE IDs have been assigned
to applications misusing the API over the years. The CVE ID was assigned to raise
awareness (and 4.1 now fixes the default behaviour as well)
https://github.com/yaml/pyyaml/pull/74

[ingydotnet]

This issue is being addressed in the upcoming 5.1 release. 5.1b1 is out, and 5.1b3 went out tonight.

The PR that resolves this issue is https://github.com/yaml/pyyaml/pull/257

[FichteFoll]

I'm not sure if issueing a "warning" is enough to solve this issue immediately. While it's of course the correct step, this is only half of the way to fix the issue eventually.

[The-Compiler]

I'm not sure if issueing a "warning" is enough to solve this issue immediately. While it's of course the correct step, this is only half of the way to fix the issue eventually.

Did you take a look at #257? From there:

We added a new loader class called FullLoader, and we made it the default for
load(). This class is almost as complete for serialization as
UnsafeLoader/Loader, but it avoids arbitrary code execution. We don't expect it
will break any code in the wild.

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.

[FichteFoll]

My bad, I must have missed that FullLoader is the new default. This is perfectly fine, then. :+1:

[ingydotnet]

PyYAML 5.1 has been released.

https://pypi.org/project/PyYAML/5.1/

This release resolves CVE-2017-18342.

For latest details, see: https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input)-Deprecation