Skip to content

Commit c2bfbcd

Browse files
miss-islingtonencukoutonghuarootrasmusfaber
authored
[3.10] gh-156002: Bound zipfile decompression for bzip2/LZMA/Zstandard (GH-156003) (GH-156362) (#156741)
* [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> * Avoid unittest's enterContext; it doesn't exist yet * 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 c1f106d commit c2bfbcd

4 files changed

Lines changed: 140 additions & 5 deletions

File tree

‎Lib/test/test_zipfile.py‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2198,6 +2198,107 @@ 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+
2238+
class MonkeypatchedDecompressorTests(unittest.TestCase):
2239+
# Some third-party projects monkey-patch _get_decompressor() to add
2240+
# additional compression schemes. This can break at any time as the
2241+
# internal compressor objects change.
2242+
# To protect users, we try to keep this case working.
2243+
# See also: GH-156002 and GH-113767.
2244+
COMPRESSION = 99
2245+
2246+
class Compressor:
2247+
"""Compressor with only the original BZ2Compressor API"""
2248+
def compress(self, data):
2249+
return data.swapcase()
2250+
2251+
def flush(self):
2252+
return b''
2253+
2254+
class Decompressor:
2255+
"""Decompressor with only the 3.3+ BZ2Decompressor API"""
2256+
eof = False
2257+
2258+
def decompress(self, data):
2259+
return data.swapcase()
2260+
2261+
def test_roundtrip_monkeypatched_decompressor(self):
2262+
orig_check_compression = zipfile._check_compression
2263+
orig_get_compressor = zipfile._get_compressor
2264+
orig_get_decompressor = zipfile._get_decompressor
2265+
2266+
def check_compression(compression):
2267+
if compression != self.COMPRESSION:
2268+
orig_check_compression(compression)
2269+
2270+
def get_compressor(compress_type, compresslevel=None):
2271+
if compress_type == self.COMPRESSION:
2272+
return self.Compressor()
2273+
return orig_get_compressor(compress_type, compresslevel)
2274+
2275+
def get_decompressor(compress_type):
2276+
if compress_type == self.COMPRESSION:
2277+
return self.Decompressor()
2278+
return orig_get_decompressor(compress_type)
2279+
2280+
with (
2281+
mock.patch.object(zipfile, '_check_compression', check_compression),
2282+
mock.patch.object(zipfile, '_get_compressor', get_compressor),
2283+
mock.patch.object(zipfile, '_get_decompressor', get_decompressor),
2284+
):
2285+
data = bytes(range(256)) * 8
2286+
buf = io.BytesIO()
2287+
with zipfile.ZipFile(buf, "w", compression=self.COMPRESSION) as zf:
2288+
zf.writestr("member", data)
2289+
self.assertIn(data.swapcase(), buf.getvalue())
2290+
with zipfile.ZipFile(io.BytesIO(buf.getvalue())) as zf:
2291+
self.assertEqual(zf.read("member"), data)
2292+
with zf.open("member") as f:
2293+
self.assertEqual(f.read(100), data[:100])
2294+
self.assertEqual(f.read1(100), data[100:200])
2295+
f.seek(-100, os.SEEK_END)
2296+
self.assertEqual(f.read(), data[-100:])
2297+
# Rewinding past the read buffer re-creates the decompressor.
2298+
f.seek(0)
2299+
self.assertEqual(f.read(), data)
2300+
2301+
22012302
class AbstractBadCrcTests:
22022303
def test_testzip_with_bad_crc(self):
22032304
"""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
@@ -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

@@ -1012,8 +1021,15 @@ def _read1(self, n):
10121021
data = self._decompressor.unconsumed_tail
10131022
if n > len(data):
10141023
data += self._read2(n - len(data))
1015-
else:
1024+
elif self._compress_type == ZIP_STORED:
10161025
data = self._read2(n)
1026+
else:
1027+
# bzip2/lzma/zstd: a bounded decompress() call may leave input
1028+
# buffered inside the decompressor; drain that before reading more.
1029+
if getattr(self._decompressor, "needs_input", True):
1030+
data = self._read2(n)
1031+
else:
1032+
data = b''
10171033

10181034
if self._compress_type == ZIP_STORED:
10191035
self._eof = self._compress_left <= 0
@@ -1026,8 +1042,17 @@ def _read1(self, n):
10261042
if self._eof:
10271043
data += self._decompressor.flush()
10281044
else:
1029-
data = self._decompressor.decompress(data)
1030-
self._eof = self._decompressor.eof or self._compress_left <= 0
1045+
# Bound the output of a single decompress() call (mirroring the
1046+
# DEFLATE path above) so that a small compressed member cannot
1047+
# expand into one unbounded read.
1048+
try:
1049+
data = self._decompressor.decompress(data, max(n, self.MIN_READ_SIZE))
1050+
except TypeError:
1051+
# See MonkeypatchedDecompressorTests in test_core.py
1052+
data = self._decompressor.decompress(data)
1053+
self._eof = (self._decompressor.eof or
1054+
self._compress_left <= 0 and
1055+
getattr(self._decompressor, "needs_input", True))
10311056

10321057
data = data[:self._left]
10331058
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)