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
The following commit(s) were added to refs/heads/master by this push:
new 7e688ebdb Avoid unbounded recursion on empty frames in framed lz4 and
snappy (#803)
7e688ebdb is described below
commit 7e688ebdba45ab16aa5b7f870474dac285a494af
Author: KALI 834X <[email protected]>
AuthorDate: Mon Aug 17 16:35:59 2026 +0530
Avoid unbounded recursion on empty frames in framed lz4 and snappy (#803)
---
.../lz4/FramedLZ4CompressorInputStream.java | 63 +++++++-------
.../snappy/FramedSnappyCompressorInputStream.java | 95 ++++++++++++----------
.../lz4/FramedLZ4CompressorInputStreamTest.java | 21 +++++
.../FramedSnappyCompressorInputStreamTest.java | 18 ++++
4 files changed, 124 insertions(+), 73 deletions(-)
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 923cc8938..11a0f1581 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
@@ -193,39 +193,46 @@ private void maybeFinishCurrentBlock() throws IOException
{
}
private void nextBlock() throws IOException {
- maybeFinishCurrentBlock();
- final long len = ByteUtils.fromLittleEndian(supplier, 4);
- final boolean uncompressed = (len & UNCOMPRESSED_FLAG_MASK) != 0;
- final int realLen = (int) (len & ~UNCOMPRESSED_FLAG_MASK);
- if (realLen == 0) {
+ while (true) {
+ maybeFinishCurrentBlock();
+ final long len = ByteUtils.fromLittleEndian(supplier, 4);
+ final boolean uncompressed = (len & UNCOMPRESSED_FLAG_MASK) != 0;
+ final int realLen = (int) (len & ~UNCOMPRESSED_FLAG_MASK);
+ if (realLen != 0) {
+ // @formatter:off
+ InputStream capped = BoundedInputStream.builder()
+ .setInputStream(inputStream)
+ .setMaxCount(realLen)
+ .setPropagateClose(false)
+ .get();
+ // @formatter:on
+ if (expectBlockChecksum) {
+ capped = new CheckedInputStream(capped, blockHash);
+ }
+ if (uncompressed) {
+ inUncompressed = true;
+ currentBlock = capped;
+ } else {
+ inUncompressed = false;
+ final BlockLZ4CompressorInputStream s = new
BlockLZ4CompressorInputStream(capped);
+ if (expectBlockDependency) {
+ s.prefill(blockDependencyBuffer);
+ }
+ currentBlock = s;
+ }
+ return;
+ }
verifyContentChecksum();
if (!decompressConcatenated) {
endReached = true;
- } else {
- init(false);
+ return;
}
- return;
- }
- // @formatter:off
- InputStream capped = BoundedInputStream.builder()
- .setInputStream(inputStream)
- .setMaxCount(realLen)
- .setPropagateClose(false)
- .get();
- // @formatter:on
- if (expectBlockChecksum) {
- capped = new CheckedInputStream(capped, blockHash);
- }
- if (uncompressed) {
- inUncompressed = true;
- currentBlock = capped;
- } else {
- inUncompressed = false;
- final BlockLZ4CompressorInputStream s = new
BlockLZ4CompressorInputStream(capped);
- if (expectBlockDependency) {
- s.prefill(blockDependencyBuffer);
+ // Start of the next concatenated frame. Loop instead of recursing
into init so a run of empty
+ // frames advances without growing the call stack.
+ if (!readSignature(false)) {
+ return;
}
- currentBlock = s;
+ readFrameDescriptor();
}
}
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 69f143e52..b3ab270b5 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
@@ -226,53 +226,58 @@ private long readCrc() throws IOException {
}
private void readNextBlock() throws IOException {
- verifyLastChecksumAndReset();
- inUncompressedChunk = false;
- final int type = readOneByte();
- if (type == -1) {
- endReached = true;
- } else if (type == STREAM_IDENTIFIER_TYPE) {
- inputStream.unread(type);
- unreadBytes++;
- pushedBackBytes(1);
- readStreamIdentifier();
- readNextBlock();
- } else if (type == PADDING_CHUNK_TYPE || type > MAX_UNSKIPPABLE_TYPE
&& type <= MAX_SKIPPABLE_TYPE) {
- skipBlock();
- readNextBlock();
- } else if (type >= MIN_UNSKIPPABLE_TYPE && type <=
MAX_UNSKIPPABLE_TYPE) {
- throw new CompressorException("Unskippable chunk with type %s (hex
%s) detected.", type, Integer.toHexString(type));
- } else if (type == UNCOMPRESSED_CHUNK_TYPE) {
- inUncompressedChunk = true;
- uncompressedBytesRemaining = readSize() - 4 /* CRC */;
- if (uncompressedBytesRemaining < 0) {
- throw new CompressorException("Found illegal chunk with
negative size");
- }
- expectedChecksum = unmask(readCrc());
- } else if (type == COMPRESSED_CHUNK_TYPE) {
- final boolean expectChecksum =
dialect.usesChecksumWithCompressedChunks();
- final long size = readSize() - (expectChecksum ? 4L : 0L);
- if (size < 0) {
- throw new CompressorException("Found illegal chunk with
negative size");
- }
- if (expectChecksum) {
- expectedChecksum = unmask(readCrc());
+ while (true) {
+ verifyLastChecksumAndReset();
+ inUncompressedChunk = false;
+ final int type = readOneByte();
+ // Stream identifiers and padding/skippable chunks produce no
output. Loop instead of recursing
+ // so a run of them advances without growing the call stack.
+ if (type == STREAM_IDENTIFIER_TYPE) {
+ inputStream.unread(type);
+ unreadBytes++;
+ pushedBackBytes(1);
+ readStreamIdentifier();
+ } else if (type == PADDING_CHUNK_TYPE || type >
MAX_UNSKIPPABLE_TYPE && type <= MAX_SKIPPABLE_TYPE) {
+ skipBlock();
} else {
- expectedChecksum = -1;
+ if (type == -1) {
+ endReached = true;
+ } else if (type >= MIN_UNSKIPPABLE_TYPE && type <=
MAX_UNSKIPPABLE_TYPE) {
+ throw new CompressorException("Unskippable chunk with type
%s (hex %s) detected.", type, Integer.toHexString(type));
+ } else if (type == UNCOMPRESSED_CHUNK_TYPE) {
+ inUncompressedChunk = true;
+ uncompressedBytesRemaining = readSize() - 4 /* CRC */;
+ if (uncompressedBytesRemaining < 0) {
+ throw new CompressorException("Found illegal chunk
with negative size");
+ }
+ expectedChecksum = unmask(readCrc());
+ } else if (type == COMPRESSED_CHUNK_TYPE) {
+ final boolean expectChecksum =
dialect.usesChecksumWithCompressedChunks();
+ final long size = readSize() - (expectChecksum ? 4L : 0L);
+ if (size < 0) {
+ throw new CompressorException("Found illegal chunk
with negative size");
+ }
+ if (expectChecksum) {
+ expectedChecksum = unmask(readCrc());
+ } else {
+ expectedChecksum = -1;
+ }
+ // @formatter:off
+ currentCompressedChunk = new
SnappyCompressorInputStream(BoundedInputStream.builder()
+ .setInputStream(inputStream)
+ .setMaxCount(size)
+ .setPropagateClose(false)
+ .get(),
+ blockSize);
+ // @formatter:on
+ // constructor reads uncompressed size
+ count(currentCompressedChunk.getBytesRead());
+ } else {
+ // impossible as all potential byte values have been
covered
+ throw new CompressorException("Unknown chunk type %s
detected.", type);
+ }
+ return;
}
- // @formatter:off
- currentCompressedChunk = new
SnappyCompressorInputStream(BoundedInputStream.builder()
- .setInputStream(inputStream)
- .setMaxCount(size)
- .setPropagateClose(false)
- .get(),
- blockSize);
- // @formatter:on
- // constructor reads uncompressed size
- count(currentCompressedChunk.getBytesRead());
- } else {
- // impossible as all potential byte values have been covered
- throw new CompressorException("Unknown chunk type %s detected.",
type);
}
}
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 c5af8c97b..63ad2c65e 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
@@ -19,6 +19,7 @@
package org.apache.commons.compress.compressors.lz4;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -27,6 +28,7 @@
import java.io.BufferedInputStream;
import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
@@ -551,6 +553,25 @@ 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 ab96fd91e..64edb039a 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
@@ -19,6 +19,7 @@
package org.apache.commons.compress.compressors.snappy;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -237,4 +238,21 @@ 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());
+ }
+ });
+ }
+
}
