grnet / djnro

DjNRO hits the decks of eduroam database management
http://djnro.grnet.gr/
Other
10 stars 21 forks source link

fix: requirements: update PyYAML to ~=5.4 #89

Closed vladimir-mencl-eresearch closed 2 years ago

vladimir-mencl-eresearch commented 2 years ago

To address CVE-2017-18342, CVE-2020-1747, and CVE-2020-14343

All related to arbitrary code execution when parsing untrusted data.

PyYAML parser is only used in extras/servdata_consumer.py

PyYAML renderer itself is also used in the servdata management command and view (when enabled).

All have been confirmed to work with the newer version.

vladimir-mencl-eresearch commented 2 years ago

Hi @zmousm , long time no see.

I hope you are doing well.

Would you be OK with merging this?

Cheers, Vlad

zmousm commented 2 years ago

Hi @vladimir-mencl-eresearch, I hope you are doing well yourself!

Yes, AFAIK djnro itself does not consume any untrusted YAML data. The extras/servdata_consumer.py script is sort-of contrib stuff not destined to run alongside the app. It only consumes yaml (or json) output from the servdata poor-man's-API endpoint.

As you probably recall we tried to follow the versions provided by the distro (namely Debian) for rather "large" requirements, such as pyyaml with LibYAML bindings. Of course this legacy approach is probably moot nowadays in the container dominated world. I don't have actual data what ~=5.4 means if you want to get LibYAML from the base distro image, but I trust you have checked this and it is fine.

So LGTM let's merge this :)

vladimir-mencl-eresearch commented 2 years ago

Hi @zmousm ,

Thanks for the quick action.

Yes, I'm aware of the aim to support distribution-provided packages - but with dependencies being fast-moving due to security vulnerabilities being discovered all the time, I saw the update more important than maintaining the support for distro-provided python packages.

And as you say, most deployments would use either containers, or some other virtual environments (ours is conda), where it would be easy to pull the exact right version.

Hope this does not break it for anyone - but staying on old versions sounds more risky...

Cheers, Vlad