fix: cap how much a ZIP entry can inflate when reading ebooks #41

Merged

jfro merged 3 commits from jtk/zip-size-caps into main

2026-10-18 17:53:32 +00:00

jfro commented

2026-10-18 17:28:55 +00:00

Owner

Closes #14.

Problem: ZipReader read whole entries into memory: the zstream path collected every chunk, and the :zip fallback used :zip.extract(..., :memory). A crafted EPUB/CBZ with a multi-GB container.xml, OPF or cover (a zip bomb, ~1000:1 with deflate) could exhaust memory and take down the BEAM during a scan.

Fix: ZipReader.read_entry/4 takes max_bytes: (default 50 MB; ZipReader.xml_max_bytes/0 = 5 MB for container.xml, the OPF and ComicInfo.xml).

zstream path: refuses early if the local header declares a bigger size, and counts inflated bytes as chunks arrive, stopping once past the cap. Headers can lie, so the count is what matters.
:zip fallback (archives with data descriptors): checks the central directory's uncompressed size first, then runs :zip.extract in a separate process with max_heap_size (3× the cap, with include_shared_binaries: true, floor 8 MB). If the size was a lie the process is killed, not the VM. It also checks byte_size of what comes back.
Refusals log a warning with the label, entry name, path and cap, and return the user-facing error "EPUB file has an entry that is too large to process: …".

Tests: zip_reader_test.exs: under/over the cap for both plain and data-descriptor archives (built with :zip.create and Zstream.zip), a streamed archive whose central directory lies about a 30 MB entry, and an EPUB whose container.xml is over 5 MB. I also checked by hand that the heap-capped process is the one that dies (:killed) on the lying archive. mix precommit: 493 passed.

Closes #14. **Problem:** `ZipReader` read whole entries into memory: the zstream path collected every chunk, and the `:zip` fallback used `:zip.extract(..., :memory)`. A crafted EPUB/CBZ with a multi-GB `container.xml`, OPF or cover (a zip bomb, ~1000:1 with deflate) could exhaust memory and take down the BEAM during a scan. **Fix:** `ZipReader.read_entry/4` takes `max_bytes:` (default 50 MB; `ZipReader.xml_max_bytes/0` = 5 MB for `container.xml`, the OPF and `ComicInfo.xml`). - **zstream path:** refuses early if the local header declares a bigger size, and counts inflated bytes as chunks arrive, stopping once past the cap. Headers can lie, so the count is what matters. - **`:zip` fallback** (archives with data descriptors): checks the central directory's uncompressed size first, then runs `:zip.extract` in a separate process with `max_heap_size` (3× the cap, with `include_shared_binaries: true`, floor 8 MB). If the size was a lie the process is killed, not the VM. It also checks `byte_size` of what comes back. - Refusals log a warning with the label, entry name, path and cap, and return the user-facing error "EPUB file has an entry that is too large to process: …". **Tests:** `zip_reader_test.exs`: under/over the cap for both plain and data-descriptor archives (built with `:zip.create` and `Zstream.zip`), a streamed archive whose central directory lies about a 30 MB entry, and an EPUB whose `container.xml` is over 5 MB. I also checked by hand that the heap-capped process is the one that dies (`:killed`) on the lying archive. `mix precommit`: 493 passed.

jfro added 1 commit

2026-10-18 17:28:56 +00:00

fix: cap how much a ZIP entry can inflate when reading ebooks

CI / test (pull_request) Successful in 42s

Details

bb8ce0a65a

ZipReader.read_entry/4 takes :max_bytes (50 MB default, 5 MB for
container.xml, OPF and ComicInfo.xml). The zstream path checks the declared
size and counts inflated bytes; the :zip fallback checks the central
directory size and extracts in a process with a heap cap, so a lying header
can't take down the BEAM.

Closes #14

jfro reviewed

2026-10-18 17:45:36 +00:00

jfro left a comment

Review from Nye: APPROVE at bb8ce0a (posted by Upper Wolfjaw; Nye's token is read-only)

CI green, merges cleanly with main and with #37–#40. Checked: Zstream 0.6.7 Entry.size is the local-header uncompressed_size, so the header check is valid and the inflated-byte counter catches lies; data-descriptor archives fall to :zip, where list_dir's file_info size is the central-dir size, and spawn_monitor + max_heap_size (include_shared_binaries, kill) catches lies there; :killed maps to :too_large; no other :zip/Zstream callers outside ZipReader.

Non-blocking lows (filed separately): (1) Zstream inflates every entry it passes while searching, so a huge entry before the target costs unbounded CPU (memory stays bounded); (2) extract_capped's receive has no after timeout, so a stuck :zip extraction could block the Oban worker forever.

**Review from Nye: APPROVE** at bb8ce0a (posted by Upper Wolfjaw; Nye's token is read-only) CI green, merges cleanly with main and with #37–#40. Checked: Zstream 0.6.7 Entry.size is the local-header uncompressed_size, so the header check is valid and the inflated-byte counter catches lies; data-descriptor archives fall to :zip, where list_dir's file_info size is the central-dir size, and spawn_monitor + max_heap_size (include_shared_binaries, kill) catches lies there; :killed maps to :too_large; no other :zip/Zstream callers outside ZipReader. Non-blocking lows (filed separately): (1) Zstream inflates every entry it passes while searching, so a huge entry before the target costs unbounded CPU (memory stays bounded); (2) extract_capped's receive has no `after` timeout, so a stuck :zip extraction could block the Oban worker forever.

jfro added 1 commit

2026-10-18 17:46:38 +00:00

Merge branch 'main' into jtk/zip-size-caps

CI / test (pull_request) Successful in 56s

Details

1deb6cc04f

jfro referenced this pull request

2026-10-18 17:46:55 +00:00

[low] ZIP reader: CPU on skipped entries, no timeout on :zip extraction #44

jfro added 1 commit

2026-10-18 17:51:25 +00:00

Merge branch 'main' into jtk/zip-size-caps