Skip to content

Commit 0ec41f6

Browse files
authored
security: bound ZIP/NPZ member decompression (#6)
* security: bound ZIP member decompression * test: cover ZIP resource limits * security: handle malformed ZIP member reads * test: cover ZIP archive member-count limit
1 parent 1f1d27a commit 0ec41f6

2 files changed

Lines changed: 85 additions & 4 deletions

File tree

‎modelhawk.py‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -369,15 +369,48 @@ def scan_npy_bytes(data: bytes, label: str = "<npy>") -> "StreamReport":
369369
return stream
370370

371371

372+
# Resource limits for untrusted ZIP/NPZ containers. Oversized entries are
373+
# surfaced as parse errors (MEDIUM) instead of silently skipped.
374+
_MAX_ARCHIVE_MEMBERS = 10_000
375+
_MAX_ARCHIVE_MEMBER_BYTES = 64 * 1024 * 1024
376+
_MAX_ARCHIVE_TOTAL_BYTES = 256 * 1024 * 1024
377+
_ARCHIVE_LIMIT_SENTINEL = b""
378+
372379
def iter_pickle_streams(path: str):
373-
"""Yield (label, bytes) for each pickle stream in a file or zip container."""
380+
"""Yield (label, bytes) for each pickle stream in a file or zip container.
381+
382+
ZIP metadata is untrusted. Bound entry count, each decompressed member, and
383+
total bytes read; never call ZipFile.read(), which materializes an entire
384+
member before the scanner can enforce a limit. Limit violations produce a
385+
malformed-stream sentinel so scan_file reports MEDIUM rather than SAFE.
386+
"""
374387
if zipfile.is_zipfile(path):
375388
try:
376389
with zipfile.ZipFile(path) as z:
377-
for info in z.infolist():
390+
total_read = 0
391+
for index, info in enumerate(z.infolist()):
378392
if info.is_dir():
379393
continue
380-
raw = z.read(info)
394+
if index >= _MAX_ARCHIVE_MEMBERS:
395+
yield "[archive-limit:member-count]", _ARCHIVE_LIMIT_SENTINEL
396+
break
397+
if info.file_size > _MAX_ARCHIVE_MEMBER_BYTES:
398+
yield f"{info.filename}[archive-limit:member-size]", _ARCHIVE_LIMIT_SENTINEL
399+
continue
400+
remaining = _MAX_ARCHIVE_TOTAL_BYTES - total_read
401+
if info.file_size > remaining:
402+
yield f"{info.filename}[archive-limit:total-size]", _ARCHIVE_LIMIT_SENTINEL
403+
break
404+
try:
405+
with z.open(info, "r") as member:
406+
raw = member.read(min(_MAX_ARCHIVE_MEMBER_BYTES, remaining) + 1)
407+
except Exception:
408+
yield f"{info.filename}[archive-read-error]", _ARCHIVE_LIMIT_SENTINEL
409+
continue
410+
if len(raw) > _MAX_ARCHIVE_MEMBER_BYTES or len(raw) > remaining:
411+
yield f"{info.filename}[archive-limit:decompressed-size]", _ARCHIVE_LIMIT_SENTINEL
412+
break
413+
total_read += len(raw)
381414
name_lower = info.filename.lower()
382415
if name_lower.endswith(_PICKLEISH_NAMES) or _looks_like_pickle(raw):
383416
yield info.filename, raw
@@ -387,7 +420,7 @@ def iter_pickle_streams(path: str):
387420
if pkl is not None:
388421
yield f"{info.filename}[object-array]", pkl
389422
except zipfile.BadZipFile:
390-
return
423+
yield "[archive-read-error]", _ARCHIVE_LIMIT_SENTINEL
391424
return
392425
with open(path, "rb") as f:
393426
yield "<pickle>", f.read()

‎tests/test_modelhawk.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import pickle
2323
import sys
2424
import tempfile
25+
import zipfile
2526

2627
sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent))
2728

@@ -226,6 +227,52 @@ def __reduce__(self):
226227
)
227228

228229

230+
def test_zip_member_resource_limits():
231+
"""ZIP scanning must bound decompression and report limit hits as suspicious."""
232+
old_member_limit = modelhawk._MAX_ARCHIVE_MEMBER_BYTES
233+
old_total_limit = modelhawk._MAX_ARCHIVE_TOTAL_BYTES
234+
old_count_limit = modelhawk._MAX_ARCHIVE_MEMBERS
235+
try:
236+
modelhawk._MAX_ARCHIVE_MEMBER_BYTES = 32
237+
modelhawk._MAX_ARCHIVE_TOTAL_BYTES = 48
238+
modelhawk._MAX_ARCHIVE_MEMBERS = 100
239+
with tempfile.TemporaryDirectory() as d:
240+
oversized = os.path.join(d, "oversized.zip")
241+
with zipfile.ZipFile(oversized, "w", compression=zipfile.ZIP_DEFLATED) as z:
242+
z.writestr("huge.pt", b"A" * 128)
243+
report = modelhawk.scan_file(oversized)
244+
assert report.verdict == "MEDIUM", (
245+
f"oversized ZIP member must be suspicious, got {report.verdict}"
246+
)
247+
assert any("archive-limit:member-size" in s.label for s in report.streams)
248+
249+
aggregate = os.path.join(d, "aggregate.zip")
250+
with zipfile.ZipFile(aggregate, "w", compression=zipfile.ZIP_STORED) as z:
251+
z.writestr("first.bin", b"A" * 24)
252+
z.writestr("second.bin", b"B" * 32)
253+
report = modelhawk.scan_file(aggregate)
254+
assert report.verdict == "MEDIUM", (
255+
f"aggregate ZIP limit must be suspicious, got {report.verdict}"
256+
)
257+
assert any("archive-limit:total-size" in s.label for s in report.streams)
258+
259+
modelhawk._MAX_ARCHIVE_TOTAL_BYTES = 1024
260+
modelhawk._MAX_ARCHIVE_MEMBERS = 1
261+
count_limited = os.path.join(d, "count-limited.zip")
262+
with zipfile.ZipFile(count_limited, "w", compression=zipfile.ZIP_STORED) as z:
263+
z.writestr("one.bin", b"A")
264+
z.writestr("two.bin", b"B")
265+
report = modelhawk.scan_file(count_limited)
266+
assert report.verdict == "MEDIUM", (
267+
f"archive member-count limit must be suspicious, got {report.verdict}"
268+
)
269+
assert any("archive-limit:member-count" in s.label for s in report.streams)
270+
finally:
271+
modelhawk._MAX_ARCHIVE_MEMBER_BYTES = old_member_limit
272+
modelhawk._MAX_ARCHIVE_TOTAL_BYTES = old_total_limit
273+
modelhawk._MAX_ARCHIVE_MEMBERS = old_count_limit
274+
275+
229276
if __name__ == "__main__":
230277
tests = [
231278
test_detection_and_no_false_positives,
@@ -237,6 +284,7 @@ def __reduce__(self):
237284
test_npy_float_array_safe,
238285
test_npy_header_bounds_and_versions,
239286
test_npy_real_numpy_object_array,
287+
test_zip_member_resource_limits,
240288
]
241289
failed = 0
242290
for fn in tests:

0 commit comments

Comments
 (0)