[GitHub] [geode] samlanning opened pull request #2948: Fix ZipSlip bug found by LGTM.com

["samlanning (GitHub)" <gi...@apache.org>]

The unsanitized path of a zip archive entry, which may
contain '..', was used directly to resolve the destination
path for the files being unzipped.

Extracting files from a malicious archive without validating
that the destination file path is within the destination
directory can cause files outside the destination directory
to be overwritten.

I've checked where this method is used, and it seems to mostly be isolated to unit tests and some CLI commands / management stuff, so felt it is likely safe to publicly open a PR to fix this, especially given that the problem is already listed [as an alert on LGTM](https://lgtm.com/projects/g/apache/geode/alerts/?mode=tree&ruleFocus=1506728586782).

cc @bschuchardt (who has fixed a number of LGTM alerts before)

*(Full disclosure: I'm part of the team behind LGTM.com)*

### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - No

- [x] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?

- [x] Is your initial contribution a single, squashed commit?

- [x] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes? - No. Existing unit tests will check that unzip() still works. I could add a unit test to ensure that an `IOException` is thrown when a zip file contains path traversal.

- [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?


[ Full content available at: https://github.com/apache/geode/pull/2948 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] samlanning commented on issue #2948: Fix ZipSlip bug found by LGTM.com

["samlanning (GitHub)" <gi...@apache.org>]

Thanks for merging @bschuchardt!

Now that you've fixed lots of the alerts, have you considered enabling the [Automated Code Review](https://lgtm.com/projects/g/apache/geode/ci/)?

[ Full content available at: https://github.com/apache/geode/pull/2948 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] samlanning commented on issue #2948: Fix ZipSlip bug found by LGTM.com

["samlanning (GitHub)" <gi...@apache.org>]

@nabarunnag replied as requested :)

[ Full content available at: https://github.com/apache/geode/pull/2948 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] bschuchardt closed pull request #2948: Fix ZipSlip bug found by LGTM.com

["bschuchardt (GitHub)" <gi...@apache.org>]

[ pull request closed by bschuchardt ]

[ Full content available at: https://github.com/apache/geode/pull/2948 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] nabarunnag commented on issue #2948: Fix ZipSlip bug found by LGTM.com

["nabarunnag (GitHub)" <gi...@apache.org>]

@samlanning https://issues.apache.org/jira/browse/INFRA-17226
There were some security concerns that are being looked into.

[ Full content available at: https://github.com/apache/geode/pull/2948 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org