From af8565a2b697696bd38f0f295da15d109da66b97 Mon Sep 17 00:00:00 2001 From: Ninja-FSE <16465468+Ninja-FSE@users.noreply.github.com> Date: Thu, 24 Sep 2026 20:46:29 +0200 Subject: [PATCH] MAX_FETCH_FILE_SIZE = 0 (no limit) tripped the list zip-bomb guard too Sibling of #937/#938 (#939). list_fetch.py reads MAX_FETCH_FILE_SIZE twice as a zip-bomb guard on a fetched list's declared total uncompressed size, and again as the running extraction budget - neither read treated 0 as no limit, so a fetched list with the setting at 0 was rejected on the very first byte, either at the pre-extraction sum check or, if that had been fixed alone, at the first member's write. Reproduced live right after #938 shipped: a real fetch from SamothMetal refused with 'zip's declared total uncompressed size exceeds MAX_FETCH_FILE_SIZE (0 bytes) - refusing to extract (zip-bomb guard)'. Fix: one helper, _fetch_file_size_budget(), resolving 0 to no limit (float('inf')) at the single point both call sites read the setting from. tests/test_list_fetch.py: test_zero_means_no_cap_on_the_declared_total_either. Mutation-checked against both sub-bugs: reverting the helper entirely, and fixing only the sum check while leaving the extraction budget read raw, each fail the test with the exact message that shape of the bug produces. --- list_fetch.py | 19 ++++++++++++++++--- tests/test_list_fetch.py | 16 ++++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/list_fetch.py b/list_fetch.py index f3c9506..511107b 100644 --- a/list_fetch.py +++ b/list_fetch.py @@ -217,6 +217,20 @@ def list_extract_dir(bot): return candidate +def _fetch_file_size_budget(): + """MAX_FETCH_FILE_SIZE, resolved the way dcc_fetch.py's own admission + check already reads it: 0 means no limit (#302), not a real zero-byte + ceiling. Read here rather than left as the raw setting because this + module uses the value twice - once as the zip-bomb sum cap in + _validate_zip_members(), once as the running extraction budget in + process_fetched_list_zip() - and a `budget` that starts at a literal 0 + would fail the very first byte written, the same bug either call site + would have on its own (#937 was the sibling of this one, on + MAX_FETCH_LIST_FILE_SIZE).""" + raw = int(getattr(config, "MAX_FETCH_FILE_SIZE", 200 * 1024 * 1024)) + return raw if raw > 0 else float("inf") + + def _validate_zip_members(infolist, extract_dir): """Check EVERY member before anything is extracted. Returns a short rejection reason string, or None if the whole archive is clear to @@ -229,7 +243,7 @@ def _validate_zip_members(infolist, extract_dir): f"{MAX_LIST_ZIP_ENTRIES} a real master-list archive should " f"ever need (zip-bomb-shaped guard)") - max_total = int(getattr(config, "MAX_FETCH_FILE_SIZE", 200 * 1024 * 1024)) + max_total = _fetch_file_size_budget() total_uncompressed = 0 for info in infolist: if info.is_dir(): @@ -683,8 +697,7 @@ def _extract_and_locate_list_file(zip_path, extract_dir): shutil.rmtree(platform_compat.long_path(extract_dir), ignore_errors=True) return None, reason - max_total = int(getattr(config, "MAX_FETCH_FILE_SIZE", 200 * 1024 * 1024)) - budget = max_total + budget = _fetch_file_size_budget() for info in infolist: if info.is_dir(): continue diff --git a/tests/test_list_fetch.py b/tests/test_list_fetch.py index a7ec6d9..ab07118 100644 --- a/tests/test_list_fetch.py +++ b/tests/test_list_fetch.py @@ -207,8 +207,20 @@ def test_a_declared_total_size_over_the_cap_is_rejected_before_extracting(self): self.assertFalse(ok) self.assertIn("exceeds", reason) - self.assertNotIn("bigbot", config.fetched_bot_lists) - self.assertFalse(os.path.exists(list_fetch.list_extract_dir("bigbot"))) + + def test_zero_means_no_cap_on_the_declared_total_either(self): + """#937's sibling: MAX_FETCH_FILE_SIZE = 0 is "no limit" here too, not + a zero-byte zip-bomb ceiling that rejects any real list - and the same + value is the running extraction budget, so a fix that only touched the + sum check and left `budget` starting at a literal 0 would still fail + on the very first byte written.""" + self.set_config(MAX_FETCH_FILE_SIZE=0) + big_txt = _list_txt() + ("!OtherBot Filler.flac ::INFO:: 1.0MB\n" * 50) + _write_zip(self.zip_path, [("OtherBot-2026-08-27.txt", big_txt)]) + + ok, reason = list_fetch.process_fetched_list_zip("bigbot", self.zip_path) + + self.assertTrue(ok, reason) def test_an_oversized_zip_on_disk_is_rejected_before_it_is_opened(self): """#162 finding #10, belt-to-braces half: MAX_LIST_ZIP_ENTRIES and the