Fix Zip Slip vulnerability in NGC private bundle download #8682
Conversation
📝 WalkthroughWalkthroughImported Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical Zip Slip vulnerability (CWE-22) in the NGC private bundle download functionality by replacing the unsafe zipfile.extractall() call with MONAI's secure _extract_zip() utility that validates member paths and prevents path traversal attacks.
Key Changes:
- Replaced direct use of
zipfile.extractall()with the safe_extract_zip()utility function - Added import for
_extract_zipfrommonai.apps.utils
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ericspod
left a comment
ericspod
left a comment
There was a problem hiding this comment.
Looks good to me, please look at the DCO resolution process, the other CICD issues are being worked on elsewhere so we'll have to wait on that.
Replaced the unsafe `zipfile.extractall()` in `_download_from_ngc_private` with MONAI's safe extraction utility. Prevents path traversal via crafted zip member paths (CWE-22). Signed-off-by: Yue (Knox) Liu <[email protected]>
for more information, see https://pre-commit.ci Signed-off-by: Yue (Knox) Liu <[email protected]>
yueyueL
force-pushed
the
dev
branch
2 times, most recently
from
f59032b to
a334b62
Compare
Signed-off-by: Yue (Knox) Liu <[email protected]>
|
/build |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@KumoLiu The blossom-ci shows success (https://github.com/Project-MONAI/MONAI/actions/runs/20713670102) but hasn't updated on the PR. Could you help resolve this? |
|
/build |
retriggered, looks like network issue. |
…NAI#8682) Replaced the unsafe `zipfile.extractall()` in `_download_from_ngc_private` with MONAI's safe extraction utility. Prevents path traversal via crafted zip member paths (CWE-22). ### Description This changes _download_from_ngc_private() to use the same safe zip extraction path as the other bundle download sources. The previous code used ZipFile.extractall() directly, which could allow Zip Slip path traversal if a malicious archive is downloaded. Now extraction validates member paths and keeps writes within the target directory. --------- Signed-off-by: Yue (Knox) Liu <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: YunLiu <[email protected]> Signed-off-by: jirka <[email protected]>
3 participants
Replaced the unsafe
zipfile.extractall()in_download_from_ngc_privatewith MONAI's safe extraction utility. Prevents path traversal via crafted zip member paths (CWE-22).Description
This changes _download_from_ngc_private() to use the same safe zip extraction path as the other bundle download sources. The previous code used ZipFile.extractall() directly, which could allow Zip Slip path traversal if a malicious archive is downloaded. Now extraction validates member paths and keeps writes within the target directory.