Skip to content

Commit 6257029

Browse files
miss-islingtonencukoutonghuarootrasmusfaber
authored
[3.11] gh-156002: Bound zipfile decompression for bzip2/LZMA/Zstandard (GH-156003) (GH-156362) (#156740)
* [3.15] gh-156002: Bound zipfile decompression for bzip2/LZMA/Zstandard (GH-156003) (GH-156362) Patch by @tonghuaroot. zipfile.ZipExtFile._read1() bounds the output of each decompress() call for DEFLATE members by passing a max_length to zlib, but for bzip2, LZMA, and Zstandard members it called decompress() with no bound. A whole compressed chunk was therefore expanded into a single allocation before the data[:self._left] clip ran, so a consumer that deliberately reads in small chunks to limit memory (for example zf.open(name).read(8192)) was silently unprotected for non-DEFLATE members. A small, spec-conformant archive member declaring a large uncompressed size could drive multi-GB peak memory. _read1() now passes a per-call bound to the non-DEFLATE decompress() (mirroring the DEFLATE branch) and drains the decompressor's internal buffer across calls by checking needs_input before reading more compressed input. zipfile's LZMADecompressor wrapper forwards max_length and exposes needs_input so the bound also holds for LZMA members. (cherry picked from commit f897dbf) (cherry picked from commit 1b424c0) Co-authored-by: Petr Viktorin <encukou@gmail.com> Co-authored-by: tonghuaroot <tonghuaroot@gmail.com> * Remove zstd test (3.14+) * gh-156002: Keep reading through monkey-patched zipfile decompressors (GH-157180) (GH-157557) (cherry picked from commit f507e69) Co-authored-by: Petr Viktorin <encukou@gmail.com> Co-authored-by: rasmusfaber <rfaber@gmail.com> * Don't use the :cve: RST role, it doesn't exist yet --------- Co-authored-by: Petr Viktorin <encukou@gmail.com> Co-authored-by: tonghuaroot <tonghuaroot@gmail.com> Co-authored-by: rasmusfaber <rfaber@gmail.com>
1 parent 2eb0c2f commit 6257029

4 files changed

Lines changed: 143 additions & 5 deletions

File tree

‎Lib/test/test_zipfile.py‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2418,6 +2418,110 @@ def tearDown(self):
24182418
unlink(TESTFN2)
24192419

24202420

2421+
class AbstractBoundedDecompressTests:
2422+
# ZipExtFile._read1() bounds the output of each decompress() call so that a
2423+
# small member declaring a large uncompressed size cannot expand into one
2424+
# unbounded read.
2425+
def test_read1_output_is_bounded(self):
2426+
buf = io.BytesIO()
2427+
with zipfile.ZipFile(buf, "w", compression=self.compression) as zf:
2428+
zf.writestr("big", b"\0" * (4 * 1024 * 1024))
2429+
with zipfile.ZipFile(io.BytesIO(buf.getvalue())) as zf:
2430+
with zf.open("big") as f:
2431+
self.assertLessEqual(len(f._read1(100)), f.MIN_READ_SIZE)
2432+
2433+
2434+
class StoredBoundedDecompressTests(AbstractBoundedDecompressTests,
2435+
unittest.TestCase):
2436+
compression = zipfile.ZIP_STORED
2437+
2438+
2439+
@requires_zlib()
2440+
class DeflateBoundedDecompressTests(AbstractBoundedDecompressTests,
2441+
unittest.TestCase):
2442+
compression = zipfile.ZIP_DEFLATED
2443+
2444+
2445+
@requires_bz2()
2446+
class Bzip2BoundedDecompressTests(AbstractBoundedDecompressTests,
2447+
unittest.TestCase):
2448+
compression = zipfile.ZIP_BZIP2
2449+
2450+
2451+
@requires_lzma()
2452+
class LzmaBoundedDecompressTests(AbstractBoundedDecompressTests,
2453+
unittest.TestCase):
2454+
compression = zipfile.ZIP_LZMA
2455+
2456+
2457+
2458+
class MonkeypatchedDecompressorTests(unittest.TestCase):
2459+
# Some third-party projects monkey-patch _get_decompressor() to add
2460+
# additional compression schemes. This can break at any time as the
2461+
# internal compressor objects change.
2462+
# To protect users, we try to keep this case working.
2463+
# See also: GH-156002 and GH-113767.
2464+
COMPRESSION = 99
2465+
2466+
class Compressor:
2467+
"""Compressor with only the original BZ2Compressor API"""
2468+
def compress(self, data):
2469+
return data.swapcase()
2470+
2471+
def flush(self):
2472+
return b''
2473+
2474+
class Decompressor:
2475+
"""Decompressor with only the 3.3+ BZ2Decompressor API"""
2476+
eof = False
2477+
2478+
def decompress(self, data):
2479+
return data.swapcase()
2480+
2481+
def setUp(self):
2482+
orig_check_compression = zipfile._check_compression
2483+
orig_get_compressor = zipfile._get_compressor
2484+
orig_get_decompressor = zipfile._get_decompressor
2485+
2486+
def check_compression(compression):
2487+
if compression != self.COMPRESSION:
2488+
orig_check_compression(compression)
2489+
2490+
def get_compressor(compress_type, compresslevel=None):
2491+
if compress_type == self.COMPRESSION:
2492+
return self.Compressor()
2493+
return orig_get_compressor(compress_type, compresslevel)
2494+
2495+
def get_decompressor(compress_type):
2496+
if compress_type == self.COMPRESSION:
2497+
return self.Decompressor()
2498+
return orig_get_decompressor(compress_type)
2499+
2500+
self.enterContext(mock.patch.object(
2501+
zipfile, '_check_compression', check_compression))
2502+
self.enterContext(mock.patch.object(
2503+
zipfile, '_get_compressor', get_compressor))
2504+
self.enterContext(mock.patch.object(
2505+
zipfile, '_get_decompressor', get_decompressor))
2506+
2507+
def test_roundtrip_monkeypatched_decompressor(self):
2508+
data = bytes(range(256)) * 8
2509+
buf = io.BytesIO()
2510+
with zipfile.ZipFile(buf, "w", compression=self.COMPRESSION) as zf:
2511+
zf.writestr("member", data)
2512+
self.assertIn(data.swapcase(), buf.getvalue())
2513+
with zipfile.ZipFile(io.BytesIO(buf.getvalue())) as zf:
2514+
self.assertEqual(zf.read("member"), data)
2515+
with zf.open("member") as f:
2516+
self.assertEqual(f.read(100), data[:100])
2517+
self.assertEqual(f.read1(100), data[100:200])
2518+
f.seek(-100, os.SEEK_END)
2519+
self.assertEqual(f.read(), data[-100:])
2520+
# Rewinding past the read buffer re-creates the decompressor.
2521+
f.seek(0)
2522+
self.assertEqual(f.read(), data)
2523+
2524+
24212525
class AbstractBadCrcTests:
24222526
def test_testzip_with_bad_crc(self):
24232527
"""Tests that files with bad CRCs return their name from testzip."""

‎Lib/zipfile.py‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,16 @@ def __init__(self):
676676
self._unconsumed = b''
677677
self.eof = False
678678

679-
def decompress(self, data):
679+
@property
680+
def needs_input(self):
681+
# While the LZMA properties header is still being buffered, more input
682+
# is required; afterwards defer to the wrapped decompressor so a bounded
683+
# decompress() call can be drained across reads.
684+
if self._decomp is None:
685+
return True
686+
return self._decomp.needs_input
687+
688+
def decompress(self, data, max_length=-1):
680689
if self._decomp is None:
681690
self._unconsumed += data
682691
if len(self._unconsumed) <= 4:
@@ -692,7 +701,7 @@ def decompress(self, data):
692701
data = self._unconsumed[4 + psize:]
693702
del self._unconsumed
694703

695-
result = self._decomp.decompress(data)
704+
result = self._decomp.decompress(data, max_length)
696705
self.eof = self._decomp.eof
697706
return result
698707

@@ -1048,8 +1057,15 @@ def _read1(self, n):
10481057
data = self._decompressor.unconsumed_tail
10491058
if n > len(data):
10501059
data += self._read2(n - len(data))
1051-
else:
1060+
elif self._compress_type == ZIP_STORED:
10521061
data = self._read2(n)
1062+
else:
1063+
# bzip2/lzma/zstd: a bounded decompress() call may leave input
1064+
# buffered inside the decompressor; drain that before reading more.
1065+
if getattr(self._decompressor, "needs_input", True):
1066+
data = self._read2(n)
1067+
else:
1068+
data = b''
10531069

10541070
if self._compress_type == ZIP_STORED:
10551071
self._eof = self._compress_left <= 0
@@ -1062,8 +1078,17 @@ def _read1(self, n):
10621078
if self._eof:
10631079
data += self._decompressor.flush()
10641080
else:
1065-
data = self._decompressor.decompress(data)
1066-
self._eof = self._decompressor.eof or self._compress_left <= 0
1081+
# Bound the output of a single decompress() call (mirroring the
1082+
# DEFLATE path above) so that a small compressed member cannot
1083+
# expand into one unbounded read.
1084+
try:
1085+
data = self._decompressor.decompress(data, max(n, self.MIN_READ_SIZE))
1086+
except TypeError:
1087+
# See MonkeypatchedDecompressorTests in test_core.py
1088+
data = self._decompressor.decompress(data)
1089+
self._eof = (self._decompressor.eof or
1090+
self._compress_left <= 0 and
1091+
getattr(self._decompressor, "needs_input", True))
10671092

10681093
data = data[:self._left]
10691094
self._left -= len(data)
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
:mod:`zipfile` again reads members through a third-party decompressor
2+
installed by monkey-patching the private ``_get_decompressor()`` to return an
3+
object that only implements old BZ2Decompressor API from Python 3.3.
4+
Note that decompressors without ``needs_input`` and two-argument
5+
``decompress()`` are vulnerable to CVE 2026-15310.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Bound the amount of data :mod:`zipfile` decompresses per read for members
2+
compressed with bzip2, LZMA, or Zstandard, matching the existing limit for
3+
deflate. A small archive member could previously expand into an unbounded
4+
allocation even when read in small chunks.

0 commit comments

Comments
 (0)