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
Assuming severity B due to pyyaml being required by repoman.
(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().
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).
Bump it and ask Toralf for a tinderbox run over revdeps (or test them yourself, as far as possible), then stabilize etc.
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.
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
Well, I would personally prefer patching out the implicit insecure APIs on the Gentoo end.
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...
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.
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?
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?
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(+)
(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!
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
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.
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 :/
Seems to affect lirc, see: https://bugs.gentoo.org/683416
I think we can stabilize now. Arches, please stabilize. Thanks.
arm64 stable
sparc stable
amd64 stable
x86 stable
hppa stable
ia64 stable
ppc64 stable
s390 stable
ppc stable
Is dev-python/ruamel-yaml affected as well?
arm stable
alpha stable. Maintainer(s), please cleanup. Security, please add it to the existing request, or file a new one.
@ maintainer(s): sh/m68k timed out. Please just set stable keywords or mask and proceed with cleanup.
sh stable
m68k stable
@maintainer(s), ok to cleanup?
(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.
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(-)
New GLSA request filed.
This issue was resolved and addressed in GLSA 202003-45 at https://security.gentoo.org/glsa/202003-45 by GLSA coordinator Thomas Deutschmann (whissi).
Reopening for cleanup, blocked at the moment.
Unable to check for sanity: > no match for package: dev-python/pyyaml-5.1
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(-)
All done, hurray! Closing.
