Page MenuHomePhabricator

[CVE-2017-18342] pyyaml vulnerability in netbox deploy repository
Closed, ResolvedPublic

Description

operations/software/netbox/deploy has a security vulnerability reported by GitHub:

Remediation

Upgrade pyyaml to version 4.2b1 or later. For example:

pyyaml>=4.2b1

Always verify the validity and compatibility of suggestions with your codebase.
Details
CVE-2017-18342
high severity
Vulnerable versions: < 4.2b1
Patched version: 4.2b1

In PyYAML before 4.1, the yaml.load() API could execute arbitrary code. In other words, yaml.safe_load() is not used.

Event Timeline

hashar added subscribers: ayounsi, Volans.

Note Netbox itself got recently upgraded to 2.5.x (T212524).

The issue comes from:

frozen-requirements.txt
PyYAML==3.13

@hashar what are you suggesting to do here? 3.13 is the latest upstream release.

My understanding is that there is no official upstream release with this "fix", only pre-releases. 4.1 has been retracted and 4.2 has only 3 beta releases from last June/July.
The fact that yaml.load() is dangerous is known since forever and everyone knows that safe_load() must be used in all cases.

I think that at most we can check that all usages of the yaml module are using safe_load() in all dependencies, as this is being included indirectly, it's not a direct dependency of Netbox. cc @crusnov

I am not quite sure, I have dig in the git history and it is a bit of a madness :(

Pr https://github.com/yaml/pyyaml/pull/74/ has 3 commits:

  • bbcf95f - Now, for py3k! (1 year, 5 months ago) <Alex Gaynor>
  • 517e83e - wtf, how did this typo happen (1 year, 5 months ago) <Alex Gaynor>
  • 7b68405 - Make pyyaml safe by default. (1 year, 5 months ago) <Alex Gaynor>

Indeed released with 4.1 which has been unreleased.

Then there is a safety-api branch which got released as 4.2b1 and has 3dc3f5f0fc41db52e894f65cc4eec9470ffdf725.

4.2b2 reverts the three commits from PR 74:

commit 1d289e820246c13d209467756eb145fae494dc3a
Author: Ingy döt Net <ingy@ingy.net>
Date: Fri Jun 29 10:04:58 2018 -0700

Reverting https://github.com/yaml/pyyaml/pull/74

Revert "Make pyyaml safe by default."

This reverts commit bbcf95fa051fdba9bbf879332e2f7999b195cf95.
This reverts commit 7b68405c81db889f83c32846462b238ccae5be80.
This reverts commit 517e83e8058e9d6850ab432ef22d84c2ac2bba5a.

But that does not include the fix from 4.2b1.

So seems to me only 4.2b1 get the fix :/

After reading the commits. PyYAML methods are:

load / dumpUnsafe
load_safe / dump_safeSafe

I assume that people just use load which is not safe. The three commits change the semantic so that load becomes an alias of safe_load and for those willing to get code executed a new danger_load is introduced. That is what the CVE is about: people missing that load can execute code and that one should almost always use safe_load.

Or as a summary:

MethodCurrentProposed fix
loadunsafesafe
safe_loadsafesafe
danger_loadN/Aunsafe

Reading again the CVE introduction:

In PyYAML before 4.1, the yaml.load() API could execute arbitrary code. In other words, yaml.safe_load() is not used.

So I guess if netbox/django etc are using safe_load and or safe_dump we are fine and the CVE can be dismissed :)

And I pasted a summary on upstream issue 207: CVE-2017-18342 status.

We might want to audit netbox for potential usage of pyyaml.load() and pyaml.dump and if any replace them with the safe versions. Then I guess just dismiss this task.

So I went through the venv for netbox and found one apparently unsafe yaml.load, in netmiko. The rest appears to be OK on a cursory glance (unless the mechanics within ruamel.yaml are unsafe also - it doesn't appear to use pyyaml as far as I can tell).

Volans claimed this task.
Volans triaged this task as Medium priority.

With the latest version we're deploying we don't have anymore netmiko as it was coming with napalm and we removed that dependency. So all good here, resolving.

And, for what is worth, that is already tracked upstream in https://github.com/ktbyers/netmiko/issues/959

Can we make this task public then, given that everything is already being tracked in public upstream issues and we're not vulnerable?

Volans changed the visibility from "Custom Policy" to "Public (No Login Required)".