update pyyaml to fix CVE-2017-18342

[vijaykramesh]

In PyYAML before 4.1, the yaml.load() API could execute arbitrary code. In other words, yaml.safe_load is not used. See https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-18342

PyYAML released 4.1 but then pulled it from PyPi. See yaml/pyyaml#207 for some discussion. https://github.com/yaml/pyyaml/tree/release/4.2 is the ongoing release branch, but nothing has been committed since July when 4.2b4 was released to PyPi

I have a project using my fork, with PyYaml updated to 4.2b4 and everything seems to be working fine.
Additionally the travis tests passed (and my project that includes mkdocs no longer triggers a vulnerability alert due to the unsafe version of PyYAML).

[facelessuser]

MkDocs uses the "unsafe" load on purpose: #1529. As MkDocs is used to pre-render the pages for a static site (you shouldn't be using this to dynmacially render pages on a live server), this isn't really a problem. See the provided link for more info.

I wonder though, as people bring this up from time to time, if it would make sense to just provide a setting to use the safe_loader to ease people's mind. With that said, you could still load unsafe code with a malicious Python Markdown extension or a malicous MkDocs extension...yeah, I don't think it really makes that much of a difference either way...

If people have access to your shell to add in bad dynamic object loading in your yaml file, they have access to add bad plugins and all sorts of things.

[vijaykramesh]

Thanks for the fast response! I looked through #1529 (sorry I missed it before submitting this) and I have the same scenario as #1529 (comment) (CI runs mkdocs, and we run a dev-only docs server inside a VPC). But as a part of the CI build we run some vulnerability detection so at least having a way to call safe_load from here will make that happier. I'll close this for now and will report back if that works.

It also sounds like the 4.2 release of pyyaml is stalled with a bunch of debate over this same change there (see yaml/pyyaml#193, it's why they pulled 4.1) so probably not a good idea to hitch mkdocs to the current 4.2 release branch.

[waylan]

@facelessuser covered this quite well. However I have a few comments:

we run a dev-only docs server inside a VPC

This is not a supported use of the dev server and therefore it is not a valid reason to make any changes. If you want to run a server in a VPC (or anywhere except a single user dev box), then you should be using mkdocs build and serve the built site with your own third party server. In that scenario, the YAML parser is in no way connected to the server, which should reduce any security concerns.

Of course, that still leaves the build "insecure", but as previously stated, no more so than plugins do. And in fact, using safe_load would disable a feature: the ability to intentionally load Python code. In fact some users want that as a feature. For example, see Python-Markdown/markdown@6f463273 were the version of the documented library is retrieved and included in the documentation by the theme template. Some might suggest attempting to make it more secure by implementing our own scaled down variant which would only accept variables which point to strings. The problem is that anyone can create a subclass which includes code in the __str__ method, so you can not really eliminate code execution unless you completely disable any ability to include anything.

as a part of the CI build we run some vulnerability detection

Then you should be telling the vulnerability detection tool to skip the check for YAML safe loading, just like you can tell a linter to skip a rule, etc. This is a known, conscious decision made with the understanding of the concerns at issue. Namely, requiring such a restriction does not make MkDocs any more secure, but in fact, breaks supported features.