-
Notifications
You must be signed in to change notification settings - Fork 314
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 - Add zip slip validation #866
Fix - Add zip slip validation #866
Conversation
bundle-ml/src/main/scala/ml/combust/bundle/serializer/FileUtil.scala
Outdated
Show resolved
Hide resolved
@David-Fadida we just shipped #864 to ready the repo for scala 2.13, which results in a merge conflict. Mind updating your PR? Thanks! |
4188b4f
to
3ac9b11
Compare
@jsleight Can you review the changes? |
@jsleight since we are a CNA, we would like to assign a CVE to this issue/fix, is that acceptable? (I'm from David's team) |
Sounds good. |
@jsleight Are you planning to publish a new release with this fix? I don't want to publish the CVE before there is a fixed version available |
Apologies, I'll try to get a new release out today. |
v0.23.1 is released. |
The POC appears to be for the PyPi ecosystem, but the advisory is solely for Maven. Is this a mistake? |
The vulnerability was in the scala code, so putting the advisory on the Maven project seems correct to me. |
@jsleight, I'm not familiar with this project, but this scala code is also used by the PyPi hosted project? If so, I'd expect the affected ecosystems in the security advisory to include all ecosystems where the project's vulnerable code can be used. This appears to be PyPi and Maven. The reason is that when a developer imports the PyPi package into their project, security tools will not flag it as vulnerable because it's not listed as an affected ecosystem. I appreciate your response & any clarifications. |
The pypi package can end up using the scala code, however users would need to explicitly install the scala package into their spark jvm. The scala code is not bundled within the python package. I /think/ you could only upgrade the scala package it would still be compatible with the old python package but not contain the CVE (don't remember 100% off hand if there were any changes to the python side in the v0.23.1 release, would need to consult the release notes) So in a pure technical sense, the pypi artifact does not contain a CVE, though I can understand the perspective that it is strongly related -- and certainly we would recommend that people upgrade both the scala and python together. I'll defer to @srmish-jfrog about how they usually tag CVE's for artifacts split across languages in this manner. |
Add zip slip validation in
FileUtil.scala