This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-compress.git
commit bfc5bb0932baed62c71ea9fabe6e46383dceda46 Author: Gary Gregory <[email protected]> AuthorDate: Mon Aug 17 07:11:54 2026 -0400 Avoid unbounded recursion on empty frames in framed lz4 and snappy decoders (#803). - Add comments. - Sort members. --- src/changes/changes.xml | 2 ++ .../lz4/FramedLZ4CompressorInputStream.java | 1 + .../snappy/FramedSnappyCompressorInputStream.java | 1 + .../lz4/FramedLZ4CompressorInputStreamTest.java | 38 +++++++++++----------- .../FramedSnappyCompressorInputStreamTest.java | 34 +++++++++---------- 5 files changed, 40 insertions(+), 36 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index f4e45f82a..944117d21 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -152,6 +152,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory, KALI 834X">[GZip] GzipCompressorInputStream now throws CompressorException instead of IllegalArgumetException/IllegalStateException.</action> <!-- FIX snappy --> <action type="fix" dev="ggregory" due-to="Stanislav Fort, Gary Gregory">[Snappy] Fix for when a valid raw Snappy stream with uncompressed size > 2 GiB used to decompress and then fail at physical EOF with a “Premature end of stream” exception instead of completing cleanly.</action> + <action type="fix" dev="ggregory" due-to="KALI 834X, Gary Gregory">[Snappy] Avoid unbounded recursion on empty frames in framed snappy decoders (#803).</action> <!-- FIX deflate64 --> <action type="fix" dev="ggregory" due-to="KALI 834X, Gary Gregory">[Deflate64] Reject invalid literal/length and distance codes in Deflate64 decoder (#785).</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">[Deflate64] Deflate64CompressorInputStream now throws CompressorException instead of IllegalArgumetException/IllegalStateException.</action> @@ -162,6 +163,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">[LZ77] LZ77Compressor.prefill(byte[]) now throws CompressorException instead of IllegalArgumetException/IllegalStateException.</action> <!-- FIX lz4 --> <action type="fix" dev="ggregory" due-to="Gary Gregory">[LZ4] BlockLZ4CompressorOutputStream now throws CompressorException instead of IllegalArgumetException/IllegalStateException.</action> + <action type="fix" dev="ggregory" due-to="KALI 834X, Gary Gregory">[LZ4] Avoid unbounded recursion on empty frames in framed lz4 decoders (#803).</action> <!-- FIX lzw --> <action type="fix" dev="ggregory" due-to="Gary Gregory">[LZW] LZWInputStream now throws CompressorException instead of IllegalArgumetException/IllegalStateException.</action> <!-- FIX general --> diff --git a/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStream.java b/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStream.java index 11a0f1581..e3f2424d2 100644 --- a/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStream.java @@ -194,6 +194,7 @@ private void maybeFinishCurrentBlock() throws IOException { private void nextBlock() throws IOException { while (true) { + // loop terminates when a real block is found or EOF is reached. maybeFinishCurrentBlock(); final long len = ByteUtils.fromLittleEndian(supplier, 4); final boolean uncompressed = (len & UNCOMPRESSED_FLAG_MASK) != 0; diff --git a/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStream.java b/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStream.java index b3ab270b5..264672160 100644 --- a/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStream.java @@ -227,6 +227,7 @@ private long readCrc() throws IOException { private void readNextBlock() throws IOException { while (true) { + // loop terminates when a real block is found or EOF is reached. verifyLastChecksumAndReset(); inUncompressedChunk = false; final int type = readOneByte(); diff --git a/src/test/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStreamTest.java b/src/test/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStreamTest.java index 63ad2c65e..2d10e6af6 100644 --- a/src/test/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStreamTest.java +++ b/src/test/java/org/apache/commons/compress/compressors/lz4/FramedLZ4CompressorInputStreamTest.java @@ -91,6 +91,25 @@ void testBackreferenceWithOffsetTooBigCausesIOException() { expectIOException("COMPRESS-490/ArrayIndexOutOfBoundsException2.lz4"); } + @Test + void testManyEmptyConcatenatedFramesDoNotOverflowStack() throws IOException { + final byte[] singleEmptyFrame; + try (ByteArrayOutputStream frame = new ByteArrayOutputStream()) { + new FramedLZ4CompressorOutputStream(frame).close(); + singleEmptyFrame = frame.toByteArray(); + } + final ByteArrayOutputStream concatenated = new ByteArrayOutputStream(); + for (int i = 0; i < 200_000; i++) { + concatenated.write(singleEmptyFrame); + } + final byte[] input = concatenated.toByteArray(); + assertDoesNotThrow(() -> { + try (FramedLZ4CompressorInputStream in = new FramedLZ4CompressorInputStream(new ByteArrayInputStream(input), true)) { + assertEquals(-1, in.read()); + } + }); + } + @Test void testMatches() throws IOException { assertFalse(FramedLZ4CompressorInputStream.matches(new byte[10], 4)); @@ -553,25 +572,6 @@ void testSkipsOverSkippableFrames() throws IOException { } } - @Test - void testManyEmptyConcatenatedFramesDoNotOverflowStack() throws IOException { - final byte[] singleEmptyFrame; - try (ByteArrayOutputStream frame = new ByteArrayOutputStream()) { - new FramedLZ4CompressorOutputStream(frame).close(); - singleEmptyFrame = frame.toByteArray(); - } - final ByteArrayOutputStream concatenated = new ByteArrayOutputStream(); - for (int i = 0; i < 200_000; i++) { - concatenated.write(singleEmptyFrame); - } - final byte[] input = concatenated.toByteArray(); - assertDoesNotThrow(() -> { - try (FramedLZ4CompressorInputStream in = new FramedLZ4CompressorInputStream(new ByteArrayInputStream(input), true)) { - assertEquals(-1, in.read()); - } - }); - } - @Test void testSkipsOverTrailingSkippableFrames() throws IOException { final byte[] input = { 4, 0x22, 0x4d, 0x18, // signature diff --git a/src/test/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStreamTest.java b/src/test/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStreamTest.java index 64edb039a..0cc93e641 100644 --- a/src/test/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStreamTest.java +++ b/src/test/java/org/apache/commons/compress/compressors/snappy/FramedSnappyCompressorInputStreamTest.java @@ -109,6 +109,23 @@ void testLoremIpsum() throws Exception { assertArrayEquals(Files.readAllBytes(outputSz), Files.readAllBytes(outputGz)); } + @Test + void testManyPaddingChunksDoNotOverflowStack() { + final byte[] streamIdentifier = { (byte) 0xff, 6, 0, 0, 's', 'N', 'a', 'P', 'p', 'Y' }; + final byte[] emptyPaddingChunk = { (byte) 0xfe, 0, 0, 0 }; + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + out.write(streamIdentifier, 0, streamIdentifier.length); + for (int i = 0; i < 200_000; i++) { + out.write(emptyPaddingChunk, 0, emptyPaddingChunk.length); + } + final byte[] input = out.toByteArray(); + assertDoesNotThrow(() -> { + try (FramedSnappyCompressorInputStream in = new FramedSnappyCompressorInputStream(new ByteArrayInputStream(input))) { + assertEquals(-1, in.read()); + } + }); + } + @Test void testMatches() throws IOException { assertFalse(FramedSnappyCompressorInputStream.matches(new byte[10], 10)); @@ -238,21 +255,4 @@ void testWriteDataLargerThanBufferOneCall() throws IOException { assertArrayEquals(data, decompressed); } - @Test - void testManyPaddingChunksDoNotOverflowStack() { - final byte[] streamIdentifier = { (byte) 0xff, 6, 0, 0, 's', 'N', 'a', 'P', 'p', 'Y' }; - final byte[] emptyPaddingChunk = { (byte) 0xfe, 0, 0, 0 }; - final ByteArrayOutputStream out = new ByteArrayOutputStream(); - out.write(streamIdentifier, 0, streamIdentifier.length); - for (int i = 0; i < 200_000; i++) { - out.write(emptyPaddingChunk, 0, emptyPaddingChunk.length); - } - final byte[] input = out.toByteArray(); - assertDoesNotThrow(() -> { - try (FramedSnappyCompressorInputStream in = new FramedSnappyCompressorInputStream(new ByteArrayInputStream(input))) { - assertEquals(-1, in.read()); - } - }); - } - }
