Skip to content

Commit e116414

Browse files
encukoutonghuaroot
authored andcommitted
[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>
1 parent 22344e9 commit e116414

3 files changed

Lines changed: 79 additions & 5 deletions

File tree

‎Lib/test/test_zipfile.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2198,6 +2198,48 @@ def tearDown(self):
21982198
unlink(TESTFN2)
21992199

22002200

2201+
class AbstractBoundedDecompressTests:
2202+
# ZipExtFile._read1() bounds the output of each decompress() call so that a
2203+
# small member declaring a large uncompressed size cannot expand into one
2204+
# unbounded read.
2205+
def test_read1_output_is_bounded(self):
2206+
buf = io.BytesIO()
2207+
with zipfile.ZipFile(buf, "w", compression=self.compression) as zf:
2208+
zf.writestr("big", b"\0" * (4 * 1024 * 1024))
2209+
with zipfile.ZipFile(io.BytesIO(buf.getvalue())) as zf:
2210+
with zf.open("big") as f:
2211+
self.assertLessEqual(len(f._read1(100)), f.MIN_READ_SIZE)
2212+
2213+
2214+
class StoredBoundedDecompressTests(AbstractBoundedDecompressTests,
2215+
unittest.TestCase):
2216+
compression = zipfile.ZIP_STORED
2217+
2218+
2219+
@requires_zlib()
2220+
class DeflateBoundedDecompressTests(AbstractBoundedDecompressTests,
2221+
unittest.TestCase):
2222+
compression = zipfile.ZIP_DEFLATED
2223+
2224+
2225+
@requires_bz2()
2226+
class Bzip2BoundedDecompressTests(AbstractBoundedDecompressTests,
2227+
unittest.TestCase):
2228+
compression = zipfile.ZIP_BZIP2
2229+
2230+
2231+
@requires_lzma()
2232+
class LzmaBoundedDecompressTests(AbstractBoundedDecompressTests,
2233+
unittest.TestCase):
2234+
compression = zipfile.ZIP_LZMA
2235+
2236+
2237+
@requires_zstd()
2238+
class ZstdBoundedDecompressTests(AbstractBoundedDecompressTests,
2239+
unittest.TestCase):
2240+
compression = zipfile.ZIP_ZSTANDARD
2241+
2242+
22012243
class AbstractBadCrcTests:
22022244
def test_testzip_with_bad_crc(self):
22032245
"""Tests that files with bad CRCs return their name from testzip."""

‎Lib/zipfile.py‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,16 @@ def __init__(self):
640640
self._unconsumed = b''
641641
self.eof = False
642642

643-
def decompress(self, data):
643+
@property
644+
def _needs_input(self):
645+
# While the LZMA properties header is still being buffered, more input
646+
# is required; afterwards defer to the wrapped decompressor so a bounded
647+
# decompress() call can be drained across reads.
648+
if self._decomp is None:
649+
return True
650+
return self._decomp.needs_input
651+
652+
def decompress(self, data, max_length=-1):
644653
if self._decomp is None:
645654
self._unconsumed += data
646655
if len(self._unconsumed) <= 4:
@@ -656,7 +665,7 @@ def decompress(self, data):
656665
data = self._unconsumed[4 + psize:]
657666
del self._unconsumed
658667

659-
result = self._decomp.decompress(data)
668+
result = self._decomp.decompress(data, max_length)
660669
self.eof = self._decomp.eof
661670
return result
662671

@@ -716,6 +725,13 @@ def _get_compressor(compress_type, compresslevel=None):
716725
return None
717726

718727

728+
def _decompressor_needs_input(decompressor):
729+
# bz2/zstd expose the stdlib decompressor's public needs_input; the LZMA
730+
# wrapper keeps it private (_needs_input) to avoid adding public API.
731+
needs_input = getattr(decompressor, "needs_input", None)
732+
return decompressor._needs_input if needs_input is None else needs_input
733+
734+
719735
def _get_decompressor(compress_type):
720736
_check_compression(compress_type)
721737
if compress_type == ZIP_STORED:
@@ -1012,8 +1028,15 @@ def _read1(self, n):
10121028
data = self._decompressor.unconsumed_tail
10131029
if n > len(data):
10141030
data += self._read2(n - len(data))
1015-
else:
1031+
elif self._compress_type == ZIP_STORED:
10161032
data = self._read2(n)
1033+
else:
1034+
# bzip2/lzma/zstd: a bounded decompress() call may leave input
1035+
# buffered inside the decompressor; drain that before reading more.
1036+
if _decompressor_needs_input(self._decompressor):
1037+
data = self._read2(n)
1038+
else:
1039+
data = b''
10171040

10181041
if self._compress_type == ZIP_STORED:
10191042
self._eof = self._compress_left <= 0
@@ -1026,8 +1049,13 @@ def _read1(self, n):
10261049
if self._eof:
10271050
data += self._decompressor.flush()
10281051
else:
1029-
data = self._decompressor.decompress(data)
1030-
self._eof = self._decompressor.eof or self._compress_left <= 0
1052+
# Bound the output of a single decompress() call (mirroring the
1053+
# DEFLATE path above) so that a small compressed member cannot
1054+
# expand into one unbounded read.
1055+
data = self._decompressor.decompress(data, max(n, self.MIN_READ_SIZE))
1056+
self._eof = (self._decompressor.eof or
1057+
self._compress_left <= 0 and
1058+
_decompressor_needs_input(self._decompressor))
10311059

10321060
data = data[:self._left]
10331061
self._left -= len(data)
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)