search

spotify / chartify

Python library that makes it easy for data scientists to create charts.
Apache License 2.0
3.53k stars 324 forks source link

Security Fix for Insecure Deserialization - huntr.dev #123

Closed huntr-helper closed 2 years ago

huntr-helper commented 3 years ago

https://huntr.dev/users/Anon-Artist has fixed the Insecure Deserialization vulnerability šŸ”Ø. Think you could fix a vulnerability like this?

Get involved at https://huntr.dev/

Q | A Version Affected | ALL Bug Fix | YES Original Pull Request | https://github.com/418sec/chartify/pull/1 Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/pip/chartify/1/README.md

User Comments:

šŸ“Š Metadata *

Arbitrary code exec vulnerability

Bounty URL: https://www.huntr.dev/bounties/1-pip-chartify

āš™ļø Description *

Chartify is a Python library that makes it easy for data scientists to create charts. This package was vulnerable to Arbitrary code execution via Insecure YAML deserialization due to the use of a known vulnerable function load() in yaml.

šŸ’» Technical Description *

This package was vulnerable to Arbitrary code execution due to the use of a known vulnerable function load() in YAML. Changing that to safe_load or using SafeLoader will fix the issue.

šŸ› Proof of Concept (PoC) *

Install the package and run the below code:

from chartify._core.colors import *

payload = '''
!!python/object/new:type
  args: ["z", !!python/tuple [], {"extend": !!python/name:exec }]
  listitems: "__import__('os').system('calc.exe')"
'''
open('pwn.yml', 'w+').write(payload)

CustomColors.from_yaml('hek', 'pwn.yml')
print("pwned šŸ“")

šŸ”„ Proof of Fix (PoF) *

After applying the fix, run the PoC again, calc wont pop and no code will be executed. Hence code exec is mitigated.

šŸ‘ User Acceptance Testing (UAT)

Only SafeLoader is used, which is the safer function, no breaking changes introduced.

danielnorberg commented 3 years ago

Unfortunately chartify users have configuration yaml files containing serialized python objects which afaik require the use of the unsafe yaml loader.

We would need to modify the configuration loading code to accept plain data and migrate users before we can switch to the safe yaml loader.

Note that there are tests in place to verify the current behavior:

https://github.com/spotify/chartify/blob/master/tests/test_colors_config.py#L20 https://github.com/spotify/chartify/blob/master/tests/test_options_config.py#L20