gh-156002: Bound zipfile decompression for bzip2/LZMA/Zstandard - #156003
Conversation
…dard 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.
Replace the Linux-only subprocess RSS test with a cross-platform check that _read1() output is bounded by MIN_READ_SIZE for bzip2/LZMA/Zstandard.
|
|
||
|
|
||
| def _decompressor_needs_input(decompressor): | ||
| # bz2/zstd expose the stdlib decompressor's public needs_input; the LZMA |
There was a problem hiding this comment.
The LZMA wrapper is private, it's not in __all__ or documented, why not just make it a "public" property and avoid this little dance?
There was a problem hiding this comment.
That's one definition of “private” :)
For the backports, I think it's best to be extra careful. Testing on 3.14.7 and having things break with 3.14.6 is not fun.
Let's make it public (& more maintainable) in 3.16 afterwards.
There was a problem hiding this comment.
This should be in Security.
|
Agreed — this bounds a decompression-bomb DoS, so |
encukou
added
needs backport to 3.10
|
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
|
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10. |
|
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
|
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
|
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
|
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.15. |
|
Sorry, @encukou, I could not cleanly backport this to |
|
Sorry, @encukou, I could not cleanly backport this to |
|
Sorry, @encukou, I could not cleanly backport this to |
|
Sorry, @encukou, I could not cleanly backport this to |
|
Sorry, @encukou, I could not cleanly backport this to |
|
Sorry, @encukou, I could not cleanly backport this to |
3 participants
Patch by @tonghuaroot