[Python-modules-team] Bug#953013: Bug#953013: pyyaml: CVE-2020-1747: arbitrary command execution through python/object/new when FullLoader is used

[Salvatore Bonaccorso]

Hi Scott,

On Sat, Mar 07, 2020 at 01:52:36AM -0500, Scott Kitterman wrote:
> On Tuesday, March 3, 2020 11:41:26 AM EST Salvatore Bonaccorso wrote:
> > Hi Scott,
> > 
> > On Tue, Mar 03, 2020 at 09:19:06AM -0500, Scott Kitterman wrote:
> > > On Tuesday, March 3, 2020 2:29:51 AM EST Salvatore Bonaccorso wrote:
> > > > Source: pyyaml
> > > > Version: 5.3-1
> > > > Severity: important
> > > > Tags: security upstream
> > > > Forwarded: https://github.com/yaml/pyyaml/pull/386
> > > > 
> > > > Hi,
> > > > 
> > > > The following vulnerability was published for pyyaml.
> > > > 
> > > > CVE-2020-1747[0]:
> > > > |arbitrary command execution through python/object/new when FullLoader
> > > > |is used
> > > > 
> > > > If you fix the vulnerability please also make sure to include the
> > > > CVE (Common Vulnerabilities & Exposures) id in your changelog entry.
> > > > 
> > > > For further information see:
> > > > 
> > > > [0] https://security-tracker.debian.org/tracker/CVE-2020-1747
> > > > 
> > > >     https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1747
> > > > 
> > > > [1] https://github.com/yaml/pyyaml/pull/386
> > > > 
> > > > Please adjust the affected versions in the BTS as needed.
> > > 
> > > Thank you for the report.
> > > 
> > > I've reviewed the current security-tracker entry and I believe it's
> > > incorrect. While it's true that the unsafe loader was the default loader
> > > in releases prior to 5.0, the safe loader has always been part of pyyaml
> > > and recommended for cases where untrusted input needed to be parsed.
> > > 
> > > Although I haven't actually tried it yet, I reviewed the relevant code
> > > from
> > > the earlier releases and I believe the same fix would work.  Unfortunately
> > > I don't have a test case to verify that.
> > > 
> > > Also, the upstream pull request to fix this is incomplete.  It only
> > > provides a fix for python3-yaml, not python-yaml.
> > > 
> > > I've updated the affected versions in the BTS and will take a shot at
> > > updating the security tracker shortly.
> > 
> > I'm actually unsure how to handle that CVE. While I believe to
> > undrstand that the unsafe loader are present, my understanding of the
> > CVE-2020-1747 is directly associated with the FullLoader use and
> > looking at the Red Hat bug where the CVE originates,
> > https://bugzilla.redhat.com/show_bug.cgi?id=1807367 it looks to me
> > that the CVE's scope is only to cover FullLoader.
> > 
> > Again I'm not really sure. But if that interpretation would be
> > correct, and given upstream recommended for for cases where untrusted
> > input needed to be parsed to use the safe loaders, then not-affected
> > would theoretically be correct, but maybe with changed/expanded
> > explanation on that FullLoader is not present until 5.1.
> > 
> > I'm not going to touch the CVE entry in the security-tracker now, and
> > looking for feedback from you all (you, Emilio, Moritz said to look as
> > well a the CVE later).
> 
> I have investigated this issue further for buster.  With the test now provided 
> in the upstream pull request added to the buster pyyaml package, the new test 
> fails (there are other failures, but those are known and can be ignored).  The 
> patch applies with only minimal adjustment.  Once the patch is applied, the 
> new test then passes.
> 
> What I can't determine is a clean way to control if the new checks are 
> enabled.  They are enabled by default in the upstream patch (which is 
> appropriate for FullLoader, which is supposed to be safe.  The Loader class in 
> the buster pyyaml is, however not.
> 
> I don't see a way around the default.  We can either make the default safe and 
> break backward compatibility or default unsafe and no one will get the benefit 
> of the improved behavior without changes to the calling code (what are the 
> odds that would happen).
> 
> This behavior is essentially what CVE-2017-18342 describes, which is open 
> against stretch and buster, so the issue is documented.
> 
> I reluctantly conclude that it's best left alone for stretch/buster and 
> because of CVE-2017-18342, the problem is reported, so we don't have any 
> additional disclosures we need to make.
> 
> I've provided the debdiff for buster in case the Security Team feels differently 
> and for anyone interested in rebuilding their pyyaml locally.

Just to quickly confirm: I thik we should as you propose leave
CVE-2017-18342 for buster and stretch actually. Good is that we will
have this sorted for bullseye and later.

Regards,
Salvatore