Can we allow salt to use PyYAML>=5?

[yjhouzz]

In the latest salt package (2019.2.1), we have requirement PyYAML<5.1.

https://github.com/saltstack/salt/blob/develop/requirements/base.txt

As a result, salt will now use PyYAML 3.x, which has known security vulnerability ( https://nvd.nist.gov/vuln/detail/CVE-2017-18342 ).

Also, due to this vulnerability, some packages have moved on to require PyYAML>=5.1 (such as googleads, but I'm sure there are others) - so they will now conflict with salt.

Can we revisit the decision and allow at least some versions of PyYAML 5.x? The commit that made this change merely says "The reason being that at least one unit test case will leak memory until exhaustion" which is somewhat vague.

• Apologies if it's already discussed - couldn't find any similar issue.

[cmcmarrow]

I think the person who made this commit believed that PyYAML leaks memory and because of this it would crash salt. I could not find any convincing evidence that confirms a PyYAML leak. I did see some chatter on this subject but again, I saw no real evidence. New versions of PyYAML takes a lot of memory and newer versions may push weaker PCs over the edge.

[cmcmarrow]

I think it would be smart to update PyYAML

[cmcmarrow]

or at best patch load

[yjhouzz]

Is there an actual verified report that PyYAML 5.x uses more memory? I couldn't find anything on https://github.com/yaml/pyyaml/issues?utf8=%E2%9C%93&q=is%3Aissue+memory

(Granted, I didn't look very hard, but still...)

Unless there's a verified issue that prevents latest version of salt from using PyYAML 5.x, I plead you to please allow it in requirements.txt to make developers' lives easier. (Please note that 3.x is widely considered insecure, as I mentioned.) You could recommend using 3.x if memory is a concern, but it seems overkill to mandate it.

[cmcmarrow]

@yjhouzz we are on the same page. If the test run green I'll get it merged.

[cmcmarrow]

Also some of are test do just import yaml and not salts yaml. salt.utils.yamlloader.load should be safer and should be used in salt.

[bgdnlp]

Please note that FreeBSD have upgraded their default py-yaml package from 3.13 to 5.1 in June, which means that Saltstack will refuse to start now with

pkg_resources.DistributionNotFound: The 'PyYAML<5.1' distribution was not found and is required by salt

It worked until this latest update.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[stale]

Thank you for updating this issue. It is no longer marked as stale.

[cmcmarrow]

PyYAML is now unlocked