Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 659348 (CVE-2017-18342) - <dev-python/pyyaml-5.1: using the yaml.load() API on untrusted input could lead to arbitrary code execution
Summary: <dev-python/pyyaml-5.1: using the yaml.load() API on untrusted input could le...
Status: RESOLVED FIXED
Alias: CVE-2017-18342
Product: Gentoo Security
Classification: Unclassified
Component: Vulnerabilities (show other bugs)
Hardware: All All
: Normal normal (vote)
Assignee: Gentoo Security
URL: https://github.com/yaml/pyyaml/pull/74
Whiteboard: B2 [glsa+ cve]
Keywords:
Depends on: 687050 687190 687192 713338 713340 713342
Blocks:
  Show dependency tree
 
Reported: 2018-06-27 11:29 UTC by Vlad K.
Modified: 2020-07-29 05:11 UTC (History)
6 users (show)

See Also:
Package list:
dev-python/pyyaml-5.1
Runtime testing required: ---
nattka: sanity-check-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vlad K. 2018-06-27 11:29:52 UTC
It is reported that in PyYAML before 4.1, usage of yaml.load() function on untrusted input could lead to arbitrary code execution. It is therefore recommended to use yaml.safe_load() instead. With 4.1, yaml.load() has been changed to call safe_load().

* Report:          http://seclists.org/oss-sec/2018/q2/240
* Upstream change: https://github.com/yaml/pyyaml/pull/74
* CVE:             pending

--

Gentoo Security Scout
Vladimir Krstulja
Comment 1 Vlad K. 2018-06-27 11:32:59 UTC
Assuming severity B due to pyyaml being required by repoman.
Comment 2 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2018-06-27 12:03:23 UTC
(In reply to Vlad K. from comment #1)
> Assuming severity B due to pyyaml being required by repoman.

Checked Portage sources already and it's using safe_load().
Comment 3 Virgil Dupras gentoo-dev 2018-08-07 01:21:57 UTC
I'd like to bump this to 4.1 because it's an important security problem, but it's likely to cause breakage all around the many consumers of this library. While (possibly) most consumers of that library used load() when in fact they only needed safe_load(), many others actually need the arbitrary serialization/deserialization that load() (which became danger_load()) provides.

Most well maintained consumers will have migrated to danger_load() by now, but other less well maintained consumers will not have.

Michał: I'd appreciate your opinion on this. What do we do, business as usual (bump, fast-stabilization, deal with breakages as they come)? Inaction isn't an option either because it's a serious issue (I think).
Comment 4 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2018-08-07 06:10:04 UTC
Bump it and ask Toralf for a tinderbox run over revdeps (or test them yourself, as far as possible), then stabilize etc.
Comment 5 Virgil Dupras gentoo-dev 2018-08-07 12:30:18 UTC
I spoke a bit fast. Looking at upstream's changelog, I could see that there was a 4.1 release in june, when trying to package it, I couldn't find it anywhere. The answer to this puzzle is at https://github.com/yaml/pyyaml/issues/207

Apparently, this release broke too many things because they retracted it. We seem to have to wait for v4.2.
Comment 6 Virgil Dupras gentoo-dev 2019-03-31 13:50:33 UTC
With PyYAML's 5.1 release, it seems that upstream doesn't want to address this CVE upfront. It went with a warning approach[1].

@security: How should we react to this. Bump to 5.1 and consider the CVE solved? Engage in tricky patching? Mask the package?

[1]: https://github.com/yaml/pyyaml/issues/193#issuecomment-457387449
Comment 7 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-03-31 16:42:04 UTC
Well, I would personally prefer patching out the implicit insecure APIs on the Gentoo end.
Comment 8 Virgil Dupras gentoo-dev 2019-03-31 16:51:25 UTC
That seems like the most sensible approach. Consumers that responsibly call load() will call python_load() anyway, so we can probably consider any load() call as insecure.

That being said, it will probably break many revdeps. Lots of revdep patching in perspective...
Comment 9 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-03-31 16:59:53 UTC
It would probably make sense to combine it with a major version bump.  Breaking API changes aren't really something that surprising in Python world.

The question is whether we should bind load() to safe_load(), or block load() completely.  The former will cause much less breakage, the latter will encourage fixing packages.
Comment 10 Virgil Dupras gentoo-dev 2019-03-31 20:37:41 UTC
What I'm preparing is a pyyaml 5.1 bump with a patch that raises an exception whenever the default load() is called. So, what would raise a warning in upstream's CVE mitigation raises an error pointing to this bug.

A quick survey of pyyaml's revdeps, however, indicate that we'll get many breakages from this. Many upstreams have already adjusted to pyyaml 5.1 by avoiding calling load() directly, but this happened only recently.

What will happen when I push this bump is that we'll be forced to bump and stabilize all revdeps or patch versions that have already been stabilized. This might prove murky.

Because of this, and because this security bug has been opened for quite a while already, I would propose that we stray a bit from our "quick stabilization + clean" policy and keep pyyaml 3.13 around for a while. When we encounter a revdep that breaks with pyyaml 5.1, we add a "<5" constraint to it and add it to a tracker bug we would create for this purpose.

Then, we try to bump and clean those revdeps as fast as possible, but maybe not with the usual eagerness associated with a regular security bump+clean.

Before you ask: why not substitute legacy load() calls with safe_load()? That would fix the CVE, but I think it would have a significant chance of introducing subtle bugs that are much harder to pin down than straight breakage. So what we'll end up with are revdeps that don't work well in some situations and it will take effort to pin down the cause the the safe_load() replacement.

What do you all think?
Comment 11 Virgil Dupras gentoo-dev 2019-04-07 23:27:35 UTC
No comments, I'll proceed. There will probably be breakage all around, but then, it's always better than a gaping hole in your system, is it?
Comment 12 Larry the Git Cow gentoo-dev 2019-04-07 23:31:25 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=79ba924d94cb0cf8559565178414c2a1d687b90c

commit 79ba924d94cb0cf8559565178414c2a1d687b90c
Author:     Virgil Dupras <vdupras@gentoo.org>
AuthorDate: 2019-04-07 23:31:14 +0000
Commit:     Virgil Dupras <vdupras@gentoo.org>
CommitDate: 2019-04-07 23:31:14 +0000

    dev-python/pyyaml: bump to 5.1
    
    Bug: https://bugs.gentoo.org/659348
    Signed-off-by: Virgil Dupras <vdupras@gentoo.org>
    Package-Manager: Portage-2.3.62, Repoman-2.3.11

 dev-python/pyyaml/Manifest                         |  1 +
 .../pyyaml/files/pyyaml-5.1-cve-2017-18342.patch   | 40 +++++++++++++++++++
 dev-python/pyyaml/pyyaml-5.1.ebuild                | 46 ++++++++++++++++++++++
 3 files changed, 87 insertions(+)
Comment 13 Aaron Bauman Gentoo Infrastructure gentoo-dev Security 2019-04-07 23:48:59 UTC
(In reply to Virgil Dupras from comment #10)
> What I'm preparing is a pyyaml 5.1 bump with a patch that raises an
> exception whenever the default load() is called. So, what would raise a
> warning in upstream's CVE mitigation raises an error pointing to this bug.
> 
> A quick survey of pyyaml's revdeps, however, indicate that we'll get many
> breakages from this. Many upstreams have already adjusted to pyyaml 5.1 by
> avoiding calling load() directly, but this happened only recently.
> 
> What will happen when I push this bump is that we'll be forced to bump and
> stabilize all revdeps or patch versions that have already been stabilized.
> This might prove murky.
> 
> Because of this, and because this security bug has been opened for quite a
> while already, I would propose that we stray a bit from our "quick
> stabilization + clean" policy and keep pyyaml 3.13 around for a while. When
> we encounter a revdep that breaks with pyyaml 5.1, we add a "<5" constraint
> to it and add it to a tracker bug we would create for this purpose.
> 
> Then, we try to bump and clean those revdeps as fast as possible, but maybe
> not with the usual eagerness associated with a regular security bump+clean.
> 
> Before you ask: why not substitute legacy load() calls with safe_load()?
> That would fix the CVE, but I think it would have a significant chance of
> introducing subtle bugs that are much harder to pin down than straight
> breakage. So what we'll end up with are revdeps that don't work well in some
> situations and it will take effort to pin down the cause the the safe_load()
> replacement.
> 
> What do you all think?

Tracking.  Let us know when you want to stabilize.

Thanks, Virgil!
Comment 14 Virgil Dupras gentoo-dev 2019-04-09 13:27:37 UTC
HOW TO FIX REVDEPS

If you're the maintainer of a revdep stumbling on this bug, be aware that your revdep uses yaml in an unsafe way. The latest version of pyyaml, as patched by Gentoo, disables unsafe calls. Here's how to fix it.

1. In all stable ebuilds, add a "<5" version constraint on the pyyaml dep. This will fix the immediate break. However, we need to get rid of these constraints soon, so we go to step 2.

2. Look at upstream's change log and see if a recent change related to pyyaml 5 was made. If yes, then you can probably remove the "<5" constraint for those versions.

3. If not, you'll need to patch out those unsafe calls. Look for calls to "yaml.load()" and replace them with calls to "yaml.full_load()" (or "yaml.safe_load()" if you know what you're doing). See upstream issue #265 [1] for details.

4. Proceed to stabilization of ebuilds without "<5" constraints so we can purge unsafe ebuilds from the tree.

[1]: https://github.com/yaml/pyyaml/issues/265
Comment 15 Virgil Dupras gentoo-dev 2019-04-09 18:01:01 UTC
I've created a tracker bug for issues that pop up from the latest 5.1 bump. Revdeps, please link your bug to the tracker bug instead of this bug to minimize noise going to the security team.
Comment 16 Bernardo Meurer 2019-04-19 04:52:44 UTC
This completely broke beets (#683814) which is a central part of my Gentoo box. Would've been nice to have gotten something in `eselect news` about it :/
Comment 17 Keith Harrison 2019-04-28 18:38:55 UTC
Seems to affect lirc, see:

https://bugs.gentoo.org/683416
Comment 18 Virgil Dupras gentoo-dev 2019-05-25 12:25:17 UTC
I think we can stabilize now. Arches, please stabilize. Thanks.
Comment 19 Aaron Bauman Gentoo Infrastructure gentoo-dev Security 2019-05-25 17:15:21 UTC
arm64 stable
Comment 20 Rolf Eike Beer 2019-05-26 08:54:31 UTC
sparc stable
Comment 21 Mikle Kolyada archtester Gentoo Infrastructure gentoo-dev Security 2019-05-26 11:39:51 UTC
amd64 stable
Comment 22 Thomas Deutschmann gentoo-dev Security 2019-05-26 22:30:01 UTC
x86 stable
Comment 23 Sergei Trofimovich gentoo-dev 2019-05-30 21:08:20 UTC
hppa stable
Comment 24 Sergei Trofimovich gentoo-dev 2019-05-30 21:09:30 UTC
ia64 stable
Comment 25 Agostino Sarubbo gentoo-dev 2019-06-04 16:59:17 UTC
ppc64 stable
Comment 26 Agostino Sarubbo gentoo-dev 2019-06-04 18:57:42 UTC
s390 stable
Comment 27 Agostino Sarubbo gentoo-dev 2019-06-05 13:10:06 UTC
ppc stable
Comment 28 Sebastian Pipping gentoo-dev 2019-06-08 22:59:17 UTC
Is dev-python/ruamel-yaml affected as well?
Comment 29 Markus Meier gentoo-dev 2019-06-13 04:26:20 UTC
arm stable
Comment 30 Agostino Sarubbo gentoo-dev 2019-06-14 09:00:34 UTC
alpha stable.

Maintainer(s), please cleanup.
Security, please add it to the existing request, or file a new one.
Comment 31 Thomas Deutschmann gentoo-dev Security 2019-10-06 19:37:23 UTC
@ maintainer(s): sh/m68k timed out. Please just set stable keywords or mask and proceed with cleanup.
Comment 32 Mikle Kolyada archtester Gentoo Infrastructure gentoo-dev Security 2019-11-02 08:06:00 UTC
sh stable
Comment 33 Mikle Kolyada archtester Gentoo Infrastructure gentoo-dev Security 2019-11-02 08:06:18 UTC
m68k stable
Comment 34 Sam James gentoo-dev Security 2020-03-19 01:45:51 UTC
@maintainer(s), ok to cleanup?
Comment 35 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2020-03-19 06:28:15 UTC
(In reply to sam_c (Security Padawan) from comment #34)
> @maintainer(s), ok to cleanup?

Cleaned what I could and opened blocker bugs for the rest.
Comment 36 Larry the Git Cow gentoo-dev 2020-03-19 06:28:53 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=bb206c8885e8085a96d32bb8346c688fb0037103

commit bb206c8885e8085a96d32bb8346c688fb0037103
Author:     Michał Górny <mgorny@gentoo.org>
AuthorDate: 2020-03-19 06:26:01 +0000
Commit:     Michał Górny <mgorny@gentoo.org>
CommitDate: 2020-03-19 06:28:46 +0000

    www-misc/urlwatch: Remove old to unblock pyyaml cleanup
    
    Bug: https://bugs.gentoo.org/659348
    Signed-off-by: Michał Górny <mgorny@gentoo.org>

 www-misc/urlwatch/Manifest             |  2 --
 www-misc/urlwatch/urlwatch-2.13.ebuild | 58 --------------------------------
 www-misc/urlwatch/urlwatch-2.16.ebuild | 60 ----------------------------------
 3 files changed, 120 deletions(-)

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=7d3bda794a85a5e34dca3249442a079cb05fc94d

commit 7d3bda794a85a5e34dca3249442a079cb05fc94d
Author:     Michał Górny <mgorny@gentoo.org>
AuthorDate: 2020-03-19 06:22:52 +0000
Commit:     Michał Górny <mgorny@gentoo.org>
CommitDate: 2020-03-19 06:28:45 +0000

    app-misc/lirc: Remove old to unblock pyyaml cleanup
    
    Bug: https://bugs.gentoo.org/659348
    Signed-off-by: Michał Górny <mgorny@gentoo.org>

 app-misc/lirc/lirc-0.10.1.ebuild | 162 ---------------------------------------
 1 file changed, 162 deletions(-)
Comment 37 Thomas Deutschmann gentoo-dev Security 2020-03-19 18:48:26 UTC
New GLSA request filed.
Comment 38 GLSAMaker/CVETool Bot gentoo-dev 2020-03-19 18:57:29 UTC
This issue was resolved and addressed in
 GLSA 202003-45 at https://security.gentoo.org/glsa/202003-45
by GLSA coordinator Thomas Deutschmann (whissi).
Comment 39 Sam James gentoo-dev Security 2020-03-26 19:52:29 UTC
Reopening for cleanup, blocked at the moment.
Comment 40 NATTkA bot gentoo-dev 2020-04-06 15:21:31 UTC
Unable to check for sanity:

> no match for package: dev-python/pyyaml-5.1
Comment 41 Larry the Git Cow gentoo-dev 2020-07-29 05:03:06 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=397831b44453de33f0abfc93dd5ed0198049cbc8

commit 397831b44453de33f0abfc93dd5ed0198049cbc8
Author:     John Helmert III <jchelmert3@posteo.net>
AuthorDate: 2020-07-26 06:33:03 +0000
Commit:     Michał Górny <mgorny@gentoo.org>
CommitDate: 2020-07-29 05:03:01 +0000

    dev-python/pyyaml: Cleanup <5.3 (security)
    
    Bug: https://bugs.gentoo.org/659348
    Package-Manager: Portage-3.0.0, Repoman-2.3.23
    Signed-off-by: John Helmert III <jchelmert3@posteo.net>
    Closes: https://github.com/gentoo/gentoo/pull/16825
    Signed-off-by: Michał Górny <mgorny@gentoo.org>

 dev-python/pyyaml/Manifest           |  1 -
 dev-python/pyyaml/pyyaml-3.13.ebuild | 41 ------------------------------------
 2 files changed, 42 deletions(-)
Comment 42 Sam James gentoo-dev Security 2020-07-29 05:11:11 UTC
All done, hurray! Closing.