search

ssato / python-anyconfig

Python library provides common APIs to load and dump configuration files in various formats
MIT License
277 stars 31 forks source link

PyYAML unsafe loading #118

Closed danielkorczak closed 2 years ago

danielkorczak commented 3 years ago

Hey @ssato,

I have a question about a snippet of code in the library, regarding how PyYAML is loaded with the ac_safe flag.

Is this line intended to wipe clear a set ac_saf opt, which is then required here to use the correct Safeloader? When testing I found that the ac_safe flag is ignored by PyYAML because L154 would reset the options variable before passing it down to yml_fnc. Ultimately, the PyYAML unsafe loader would always be used, regardless of how I'd pass down my flags.

Thanks! Dan

ssato commented 3 years ago

Is this line intended to wipe clear a set ac_saf opt, which is then required here to use the correct Safeloader? When testing I found that the ac_safe flag is ignored by PyYAML because L154 would reset the options variable before passing it down to yml_fnc.

Thanks to point it out! You're absolutely correct. I pushed the change may resolve this. Please take a look at the commit 4ab8770.

Ultimately, the PyYAML unsafe loader would always be used, regardless of how I'd pass down my flags.

I agree with you basically but want to keep as it is and leave that decision to users now.

danielkorczak commented 3 years ago

Anytime. I'm happy to help out such a great piece of software! Two other related questions:

  1. Is there a release you're targeting to get this fix into?
  2. Can a Github Security Advisory be created for this issue, so ultimately a CVE is assigned? This would help others track and remediate this issue in their projects, especially where automated dependency scanners are involved. Github makes this process easy, especially considering you already implemented a fix! I would start this process myself, but only project admins can create and publish an advisory.

https://docs.github.com/en/free-pro-team@latest/github/managing-security-vulnerabilities/creating-a-security-advisory

https://docs.github.com/en/free-pro-team@latest/github/managing-security-vulnerabilities/publishing-a-security-advisory

ssato commented 3 years ago

Is there a release you're targeting to get this fix into?

Sorry to late. I'll release the next version very soon.

Can a Github Security Advisory be created for this issue, so ultimately a CVE is assigned?

Of course!

ssato commented 3 years ago

FYI. I released 0.10.0 contains the change.