|Summary:||<dev-python/pyyaml-5.1: using the yaml.load() API on untrusted input could lead to arbitrary code execution|
|Product:||Gentoo Security||Reporter:||Vlad K. <vk-gentoo-bugs>|
|Component:||Vulnerabilities||Assignee:||Gentoo Security <security>|
|Severity:||normal||CC:||dan, mgorny, mmokrejs, python, sping, vdupras|
|Whiteboard:||B2 [cleanup blocked glsa+ cve]|
|Runtime testing required:||---|
|Bug Depends on:||687050, 687190, 687192, 713338, 713340, 713342|
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 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 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 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 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 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. @security: How should we react to this. Bump to 5.1 and consider the CVE solved? Engage in tricky patching? Mask the package? : https://github.com/yaml/pyyaml/issues/193#issuecomment-457387449
Comment 7 Michał Górny 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 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 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 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 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 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 <email@example.com> AuthorDate: 2019-04-07 23:31:14 +0000 Commit: Virgil Dupras <firstname.lastname@example.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 <email@example.com> 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 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 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  for details. 4. Proceed to stabilization of ebuilds without "<5" constraints so we can purge unsafe ebuilds from the tree. : https://github.com/yaml/pyyaml/issues/265
Comment 15 Virgil Dupras 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 2019-05-25 12:25:17 UTC
I think we can stabilize now. Arches, please stabilize. Thanks.
Comment 19 Aaron Bauman 2019-05-25 17:15:21 UTC
Comment 20 Rolf Eike Beer 2019-05-26 08:54:31 UTC
Comment 21 Mikle Kolyada 2019-05-26 11:39:51 UTC
Comment 22 Thomas Deutschmann 2019-05-26 22:30:01 UTC
Comment 23 Sergei Trofimovich 2019-05-30 21:08:20 UTC
Comment 24 Sergei Trofimovich 2019-05-30 21:09:30 UTC
Comment 25 Agostino Sarubbo 2019-06-04 16:59:17 UTC
Comment 26 Agostino Sarubbo 2019-06-04 18:57:42 UTC
Comment 27 Agostino Sarubbo 2019-06-05 13:10:06 UTC
Comment 28 Sebastian Pipping 2019-06-08 22:59:17 UTC
Is dev-python/ruamel-yaml affected as well?
Comment 29 Markus Meier 2019-06-13 04:26:20 UTC
Comment 30 Agostino Sarubbo 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 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 2019-11-02 08:06:00 UTC
Comment 33 Mikle Kolyada 2019-11-02 08:06:18 UTC
Comment 34 Sam James (sec padawan) 2020-03-19 01:45:51 UTC
@maintainer(s), ok to cleanup?
Comment 35 Michał Górny 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 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 <firstname.lastname@example.org> AuthorDate: 2020-03-19 06:26:01 +0000 Commit: Michał Górny <email@example.com> 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 <firstname.lastname@example.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 <email@example.com> AuthorDate: 2020-03-19 06:22:52 +0000 Commit: Michał Górny <firstname.lastname@example.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 <email@example.com> app-misc/lirc/lirc-0.10.1.ebuild | 162 --------------------------------------- 1 file changed, 162 deletions(-)
Comment 37 Thomas Deutschmann 2020-03-19 18:48:26 UTC
New GLSA request filed.
Comment 38 GLSAMaker/CVETool Bot 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 (sec padawan) 2020-03-26 19:52:29 UTC
Reopening for cleanup, blocked at the moment.
Comment 40 NATTkA bot 2020-04-06 15:21:31 UTC
Unable to check for sanity: > no match for package: dev-python/pyyaml-5.1