-
Notifications
You must be signed in to change notification settings - Fork 129
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
Change the ziputil dependency to fix a potential security concern #824
Conversation
Signed-off-by: Sicheng Song <sicheng.song@outlook.com>
Signed-off-by: Sicheng Song <sicheng.song@outlook.com>
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## 2.x #824 +/- ##
============================================
- Coverage 85.14% 84.82% -0.32%
- Complexity 1624 1630 +6
============================================
Files 135 136 +1
Lines 6003 6082 +79
Branches 580 596 +16
============================================
+ Hits 5111 5159 +48
- Misses 646 666 +20
- Partials 246 257 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 23 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Sicheng Song <sicheng.song@outlook.com>
ml-algorithms/src/main/java/org/opensearch/ml/engine/utils/ZipUtils.java
Outdated
Show resolved
Hide resolved
ml-algorithms/src/main/java/org/opensearch/ml/engine/utils/ZipUtils.java
Outdated
Show resolved
Hide resolved
ml-algorithms/src/main/java/org/opensearch/ml/engine/utils/ZipUtils.java
Show resolved
Hide resolved
ml-algorithms/src/test/java/org/opensearch/ml/engine/utils/ZipUtilsTest.java
Show resolved
Hide resolved
ml-algorithms/src/main/java/org/opensearch/ml/engine/utils/ZipUtils.java
Outdated
Show resolved
Hide resolved
ml-algorithms/src/main/java/org/opensearch/ml/engine/utils/ZipUtils.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Sicheng Song <sicheng.song@outlook.com>
ml-algorithms/src/main/java/org/opensearch/ml/engine/utils/ZipUtils.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Sicheng Song <sicheng.song@outlook.com>
Signed-off-by: Sicheng Song <sicheng.song@outlook.com>
ml-algorithms/src/main/java/org/opensearch/ml/engine/utils/ZipUtils.java
Outdated
Show resolved
Hide resolved
String name = ze.getName(); | ||
Path f = dest.resolve(name).toAbsolutePath(); | ||
if (!f.normalize().startsWith(dest)) | ||
throw new RuntimeException("Bad zip entry"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you write a test case to cover this branch?
Per my understanding, this line Path f = dest.resolve(name).toAbsolutePath();
already make sure f will start with dest
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, I guess the edge case should be like this one https://codeql.github.com/codeql-query-help/java/java-zipslip/#recommendation
If zip entry has name ..\sneaky-file
, then we may unzip to a parent folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but actually this edge case is really hard to create, linux does not allow me to do that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, then it's fine. you can add some todo here, you can do more research and add test later
ml-algorithms/src/main/java/org/opensearch/ml/engine/utils/ZipUtils.java
Show resolved
Hide resolved
ml-algorithms/src/test/java/org/opensearch/ml/engine/utils/ZipUtilsTest.java
Show resolved
Hide resolved
Signed-off-by: Sicheng Song <sicheng.song@outlook.com>
…ensearch-project#824) Signed-off-by: Yaliang Wu <ylwu@amazon.com>
* rename model meta/chunk API (#827) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * Change the ziputil dependency to fix a potential security concern (#824) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * add text docs ML input (#830) Signed-off-by: Yaliang Wu <ylwu@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> Co-authored-by: Sicheng Song <sicheng.song@outlook.com>
…roject#837) * rename model meta/chunk API (opensearch-project#827) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * Change the ziputil dependency to fix a potential security concern (opensearch-project#824) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * add text docs ML input (opensearch-project#830) Signed-off-by: Yaliang Wu <ylwu@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> Co-authored-by: Sicheng Song <sicheng.song@outlook.com>
* Backport changes to model-access-control feature branch (#837) * rename model meta/chunk API (#827) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * Change the ziputil dependency to fix a potential security concern (#824) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * add text docs ML input (#830) Signed-off-by: Yaliang Wu <ylwu@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> Co-authored-by: Sicheng Song <sicheng.song@outlook.com> * add model group (#840) * add model group Signed-off-by: Yaliang Wu <ylwu@amazon.com> * rename create model group as register model group Signed-off-by: Yaliang Wu <ylwu@amazon.com> * fix class name in build.gradle Signed-off-by: Yaliang Wu <ylwu@amazon.com> * remove unused code; stash thread context Signed-off-by: Yaliang Wu <ylwu@amazon.com> * exclude class for low coverage Signed-off-by: Yaliang Wu <ylwu@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> * access validation for register/update model-group, register/get/delete/deploy/predict model Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * changes to security utils and register/search model group Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * update last_updated_time in model group when new version added Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix undeploy model action Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix format violations Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * rebased to 2.x and prediction/search API fix Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix rebase conflicts Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * new delete model group API Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix formal violations Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> Co-authored-by: Yaliang Wu <ylwu@amazon.com> Co-authored-by: Sicheng Song <sicheng.song@outlook.com>
* Backport changes to model-access-control feature branch (opensearch-project#837) * rename model meta/chunk API (opensearch-project#827) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * Change the ziputil dependency to fix a potential security concern (opensearch-project#824) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * add text docs ML input (opensearch-project#830) Signed-off-by: Yaliang Wu <ylwu@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> Co-authored-by: Sicheng Song <sicheng.song@outlook.com> * add model group (opensearch-project#840) * add model group Signed-off-by: Yaliang Wu <ylwu@amazon.com> * rename create model group as register model group Signed-off-by: Yaliang Wu <ylwu@amazon.com> * fix class name in build.gradle Signed-off-by: Yaliang Wu <ylwu@amazon.com> * remove unused code; stash thread context Signed-off-by: Yaliang Wu <ylwu@amazon.com> * exclude class for low coverage Signed-off-by: Yaliang Wu <ylwu@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> * access validation for register/update model-group, register/get/delete/deploy/predict model Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * changes to security utils and register/search model group Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * update last_updated_time in model group when new version added Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix undeploy model action Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix format violations Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * rebased to 2.x and prediction/search API fix Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix rebase conflicts Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * new delete model group API Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix formal violations Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> Co-authored-by: Yaliang Wu <ylwu@amazon.com> Co-authored-by: Sicheng Song <sicheng.song@outlook.com>
* Backport changes to model-access-control feature branch (opensearch-project#837) * rename model meta/chunk API (opensearch-project#827) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * Change the ziputil dependency to fix a potential security concern (opensearch-project#824) Signed-off-by: Yaliang Wu <ylwu@amazon.com> * add text docs ML input (opensearch-project#830) Signed-off-by: Yaliang Wu <ylwu@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> Co-authored-by: Sicheng Song <sicheng.song@outlook.com> * add model group (opensearch-project#840) * add model group Signed-off-by: Yaliang Wu <ylwu@amazon.com> * rename create model group as register model group Signed-off-by: Yaliang Wu <ylwu@amazon.com> * fix class name in build.gradle Signed-off-by: Yaliang Wu <ylwu@amazon.com> * remove unused code; stash thread context Signed-off-by: Yaliang Wu <ylwu@amazon.com> * exclude class for low coverage Signed-off-by: Yaliang Wu <ylwu@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> * access validation for register/update model-group, register/get/delete/deploy/predict model Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * changes to security utils and register/search model group Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * update last_updated_time in model group when new version added Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix undeploy model action Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix format violations Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * rebased to 2.x and prediction/search API fix Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix rebase conflicts Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * new delete model group API Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> * fix formal violations Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> --------- Signed-off-by: Yaliang Wu <ylwu@amazon.com> Signed-off-by: Bhavana Ramaram <rbhavna@amazon.com> Co-authored-by: Yaliang Wu <ylwu@amazon.com> Co-authored-by: Sicheng Song <sicheng.song@outlook.com> Signed-off-by: Sicheng Song <sicheng.song@outlook.com>
Description
We've written our own ZipUtil class, where unzip method used Apache Commons’ ZipFile classes instead of ZipInputStream class. This can fix a potential security vulnerability.
Issues Resolved
This resolved a sev3 ticket.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.