Skip to content

Add zip bomb protection to ZipConverter#1628

Open
mvanhorn wants to merge 1 commit intomicrosoft:mainfrom
mvanhorn:fix/zip-bomb-dos-protection
Open

Add zip bomb protection to ZipConverter#1628
mvanhorn wants to merge 1 commit intomicrosoft:mainfrom
mvanhorn:fix/zip-bomb-dos-protection

Conversation

@mvanhorn
Copy link
Copy Markdown

Problem

ZipConverter reads each file in a ZIP archive by calling zipObj.read(name) without checking the decompressed size first. A malicious zip bomb (a small ZIP that decompresses to gigabytes) can crash the process with an out-of-memory error.

Reported in #1514.

Fix

Added three pre-read safety checks in ZipConverter.convert():

  1. Per-file size limit (100 MB) - checks ZipInfo.file_size before reading
  2. Decompression ratio limit (100:1) - compares file_size / compress_size to detect suspiciously high compression ratios typical of zip bombs
  3. Cumulative size limit (500 MB) - tracks total decompressed bytes across all files in the archive

Files exceeding any limit are skipped with a logging.warning() message rather than raising an exception, so the rest of the archive is still processed (graceful degradation).

Testing

Added test_zip_security.py with four tests:

  • Normal ZIP converts successfully (no regression)
  • Oversized individual file is skipped
  • High decompression ratio is detected and skipped
  • Cumulative size limit stops extraction

All limits are defined as module-level constants (MAX_DECOMPRESSED_FILE_SIZE, MAX_DECOMPRESSION_RATIO, MAX_TOTAL_DECOMPRESSED_SIZE) for easy adjustment.

This contribution was developed with AI assistance (Claude Code).

Check ZipInfo.file_size and decompression ratio before reading each
entry. Skip files that exceed the per-file size limit (100 MB),
decompression ratio limit (100:1), or cumulative total (500 MB).
Exceeding limits logs a warning instead of crashing.

Fixes microsoft#1514
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant