Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: avoid CVE-2017-18342 #4292

Merged
merged 1 commit into from
Jan 19, 2021
Merged

Conversation

hoxnox
Copy link
Contributor

@hoxnox hoxnox commented Jan 18, 2021

boost/1.73.0

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

Usage of pyyaml::load is unsafe.
Gentoo blocks these calls if pyyaml installed through portage.
See https://bugs.gentoo.org/659348

@conan-center-bot
Copy link
Collaborator

All green in build 1 (04c5e7deee9442f388c5826e69303c141a340416)! 😊

Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to easy... what's the side effect? why did they make a new function instead of fixing the old one?

@hoxnox
Copy link
Contributor Author

hoxnox commented Jan 19, 2021

Looks to easy... what's the side effect? why did they make a new function instead of fixing the old one?

In this particular case there is no side effects. The author of pyyml deside to fix possible RCE with new function safe_load and warning in load. Gentoo community deside to block this function in the portage version of the library.

@SSE4
Copy link
Contributor

SSE4 commented Jan 19, 2021

maybe we should block unsafe python calls via hooks as well, to prevent such situations?
are there other examples besides yaml.load? is there a list available somewhere?

@hoxnox
Copy link
Contributor Author

hoxnox commented Jan 19, 2021

is there a list available somewhere?

I don't know such lists.
Maybe there is a security linter for python.
Or static vulnerability scanner.

But I think this is overkill in this software. Conan recipes checks what they download to execute through checksums.

In this particular case I didn't care about security. My gentoo system fails with error with this recipe if conan installed through portage system with portage dependecies (pyyaml in gentoo patched).

@SSE4
Copy link
Contributor

SSE4 commented Jan 19, 2021

we have two occurances now:

./recipes/boost/all/conanfile.py:165:        return yaml.load(open(dependencies_filepath))
./recipes/open62541/all/conanfile.py:170:            submodules_data = yaml.load(submodule_stream)

need to open PR for open62541 as well

@conan-center-bot
Copy link
Collaborator

All green in build 2 (04c5e7deee9442f388c5826e69303c141a340416)! 😊

@hoxnox
Copy link
Contributor Author

hoxnox commented Jan 19, 2021

we have two occurances now:

./recipes/boost/all/conanfile.py:165:        return yaml.load(open(dependencies_filepath))
./recipes/open62541/all/conanfile.py:170:            submodules_data = yaml.load(submodule_stream)

need to open PR for open62541 as well

That good news, I thought there will be much more.
Later I'll make a PR. Thanks.

@prince-chrismc
Copy link
Contributor

👍 https://bugs.gentoo.org/659348

@prince-chrismc
Copy link
Contributor

prince-chrismc commented Jan 19, 2021

Now I am confused, safe_load === load 🤔

https://github.com/yaml/pyyaml/pull/74/files

@conan-center-bot conan-center-bot merged commit b2c56c2 into conan-io:master Jan 19, 2021
@Croydon
Copy link
Contributor

Croydon commented Jan 19, 2021

Now I am confused, safe_load === load 🤔

https://github.com/yaml/pyyaml/pull/74/files

Looks like this is only a problem with old pyyaml versions then. Hence, I think we have no need for another hook for this

@hoxnox
Copy link
Contributor Author

hoxnox commented Jan 19, 2021

Now I am confused, safe_load === load thinking
https://github.com/yaml/pyyaml/pull/74/files

Looks like this is only a problem with old pyyaml versions then. Hence, I think we have no need for another hook for this

yah, I should try to force gentoo comunity fix ebuild in portage

tail -n 19 pyyaml-5.3.1-r1.ebuild | head -n 6

PATCHES=(
        # bug #659348
        "${FILESDIR}/pyyaml-5.1-cve-2017-18342.patch"
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants