diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 3e7f5463e12..af7bf4aa47a 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -56,6 +56,7 @@ The type attribute can be add,update,fix,remove. AES256SHA256Decoder now enforces the CPP source k_NumCyclesPower_Supported_MAX = 24 limit. Don't loose precision while reading folders from a SevenZFile. Improve some exception messages in TarUtils and TarArchiveEntry. + SevenZFile now enforces the same folder and coder limits as the CPP implementation. BZip2CompressorInputStream now throw CompressorException (a subclass of IOException) for invalid or corrupted data, providing more specific error reporting. BZip2 input streams treat Huffman codes longer than 20 bits as corrupted data, matching the behavior of the reference implementation. diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/Folder.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/Folder.java index fca70b07395..7e368655887 100644 --- a/src/main/java/org/apache/commons/compress/archivers/sevenz/Folder.java +++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/Folder.java @@ -37,13 +37,23 @@ final class Folder { /** * Total number of input streams across all coders. This field is currently unused but technically part of the 7z API. + * + *

Currently limited to {@code MAX_CODER_STREAMS_PER_FOLDER}

*/ - long totalInputStreams; + int totalInputStreams; - /** Total number of output streams across all coders. */ - long totalOutputStreams; + /** + * Total number of output streams across all coders. + * + *

Currently limited to {@code MAX_CODER_STREAMS_PER_FOLDER}

+ */ + int totalOutputStreams; - /** Mapping between input and output streams. */ + /** + * Mapping between input and output streams. + * + *

Its size is equal to {@code totalOutputStreams - 1}

+ */ BindPair[] bindPairs; /** Indices of input streams, one per input stream not listed in bindPairs. */ diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java index a6cdf6bc3a5..112e5fe5f7a 100644 --- a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java @@ -339,6 +339,30 @@ public Builder setUseDefaultNameForUnnamedEntries(final boolean useDefaultNameFo /** Shared with SevenZOutputFile and tests, neither mutates it. */ static final byte[] SIGNATURE = { (byte) '7', (byte) 'z', (byte) 0xBC, (byte) 0xAF, (byte) 0x27, (byte) 0x1C }; + /** + * Maximum number of coders permitted in a single 7z folder. + * + *

This limit is defined by the original 7-Zip implementation + * ({@code CPP/7zip/Archive/7z/7zIn.cpp}) to guard against malformed archives:

+ * + * 
+     * #define k_Scan_NumCoders_MAX 64
+     *
+ */ + private static final int MAX_CODERS_PER_FOLDER = 64; + + /** + * Maximum total number of coder input/output streams permitted in a single folder. + * + *

This limit is also taken from the reference implementation + * ({@code CPP/7zip/Archive/7z/7zIn.cpp}):

+ * + * 
+     * #define k_Scan_NumCodersStreams_in_Folder_MAX 64
+     *
+ */ + private static final int MAX_CODER_STREAMS_PER_FOLDER = 64; + /** * Creates a new Builder. * @@ -1438,12 +1462,15 @@ private void readFilesInfo(final ByteBuffer header, final Archive archive) throw calculateStreamMap(archive); } - private Folder readFolder(final ByteBuffer header) throws IOException { + Folder readFolder(final ByteBuffer header) throws IOException { final Folder folder = new Folder(); - final int numCoders = readUint64ToIntExact(header); - final Coder[] coders = new Coder[checkObjectArray(numCoders)]; - long totalInStreams = 0; - long totalOutStreams = 0; + final long numCoders = readUint64(header); + if (numCoders == 0 || numCoders > MAX_CODERS_PER_FOLDER) { + throw new ArchiveException("Unsupported 7z archive: " + numCoders + " coders in folder."); + } + final Coder[] coders = new Coder[(int) numCoders]; + int totalInStreams = 0; + int totalOutStreams = 0; for (int i = 0; i < coders.length; i++) { final int bits = getUnsignedByte(header); final int idSize = bits & 0xf; @@ -1459,10 +1486,19 @@ private Folder readFolder(final ByteBuffer header) throws IOException { numOutStreams = 1; } else { numInStreams = readUint64(header); + if (numInStreams > MAX_CODER_STREAMS_PER_FOLDER) { + throw new ArchiveException("Unsupported 7z archive: %,d coder input streams in folder.", numInStreams); + } numOutStreams = readUint64(header); + if (numOutStreams != 1) { + throw new ArchiveException("Unsupported 7z archive: %,d coder output streams in folder.", numOutStreams); + } + } + totalInStreams += (int) numInStreams; + if (totalInStreams > MAX_CODER_STREAMS_PER_FOLDER) { + throw new ArchiveException("Unsupported 7z archive: %,d coder input streams in folder.", totalInStreams); } - totalInStreams = ArchiveException.addExact(totalInStreams, numInStreams); - totalOutStreams = ArchiveException.addExact(totalOutStreams, numOutStreams); + totalOutStreams += (int) numOutStreams; byte[] properties = null; if (hasAttributes) { final long propertiesSize = readUint64(header); @@ -1471,22 +1507,30 @@ private Folder readFolder(final ByteBuffer header) throws IOException { } // would need to keep looping as above: if (moreAlternativeMethods) { - throw new ArchiveException("Alternative methods are unsupported, please report. The reference implementation doesn't support them either."); + throw new ArchiveException("Unsupported 7z archive: alternative methods are unsupported, please report. " + + "The reference implementation doesn't support them either."); } coders[i] = new Coder(decompressionMethodId, numInStreams, numOutStreams, properties); } folder.coders = coders; folder.totalInputStreams = totalInStreams; folder.totalOutputStreams = totalOutStreams; - final long numBindPairs = totalOutStreams - 1; - final BindPair[] bindPairs = new BindPair[checkObjectArray(ArchiveException.toIntExact(numBindPairs))]; + final int numBindPairs = totalOutStreams - 1; + final BindPair[] bindPairs = new BindPair[numBindPairs]; for (int i = 0; i < bindPairs.length; i++) { - bindPairs[i] = new BindPair(readUint64(header), readUint64(header)); + final long inIndex = readUint64(header); + if (inIndex >= totalInStreams) { + throw new ArchiveException("Unsupported 7z archive: bind pair inIndex %d out of range.", inIndex); + } + final long outIndex = readUint64(header); + if (outIndex >= totalOutStreams) { + throw new ArchiveException("Unsupported 7z archive: bind pair outIndex %d out of range.", inIndex); + } + bindPairs[i] = new BindPair(inIndex, outIndex); } folder.bindPairs = bindPairs; - final long numPackedStreams = totalInStreams - numBindPairs; - final int numPackedStreamsInt = ArchiveException.toIntExact(numPackedStreams); - final long[] packedStreams = new long[checkObjectArray(numPackedStreamsInt)]; + final int numPackedStreams = totalInStreams - numBindPairs; + final long[] packedStreams = new long[numPackedStreams]; if (numPackedStreams == 1) { long i; for (i = 0; i < totalInStreams; i++) { @@ -1496,8 +1540,11 @@ private Folder readFolder(final ByteBuffer header) throws IOException { } packedStreams[0] = i; } else { - for (int i = 0; i < numPackedStreamsInt; i++) { + for (int i = 0; i < numPackedStreams; i++) { packedStreams[i] = readUint64(header); + if (packedStreams[i] >= totalInStreams) { + throw new ArchiveException("Unsupported 7z archive: packed stream index %d out of range.", packedStreams[i]); + } } } folder.packedStreams = packedStreams; diff --git a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java index ec3878429a3..772a2101f90 100644 --- a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java +++ b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java @@ -33,6 +33,8 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.FileTime; @@ -47,7 +49,9 @@ import java.util.List; import java.util.Map; import java.util.Random; +import java.util.function.Consumer; import java.util.function.Function; +import java.util.stream.Stream; import javax.crypto.Cipher; @@ -61,6 +65,8 @@ import org.apache.commons.io.input.ChecksumInputStream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; class SevenZFileTest extends AbstractArchiveFileTest { private static final String TEST2_CONTENT = "\r\n\r\n\r\n\t\r\n\n"; @@ -69,6 +75,114 @@ private static boolean isStrongCryptoAvailable() throws NoSuchAlgorithmException return Cipher.getMaxAllowedKeyLength("AES/ECB/PKCS5Padding") >= 256; } + static Stream> testReadFolder_Unsupported() { + return Stream.of( + // Folder with no coders + buf -> writeFolder(buf, new Coder[0]), + // Folder with too many coders + buf -> { + final Coder[] coders = new Coder[65]; + final Coder simpleCoder = new Coder(new byte[] { 0x03 }, 1, 1, null); + Arrays.fill(coders, simpleCoder); + writeFolder(buf, coders); + }, + // Folder with too many input streams per coder + buf -> { + final Coder coder = new Coder(new byte[] { 0x03 }, 65, 1, null); + writeFolder(buf, new Coder[] { coder }); + }, + // Folder with more than one output stream per coder + buf -> { + final Coder coder = new Coder(new byte[] { 0x03 }, 1, 2, null); + writeFolder(buf, new Coder[] { coder }); + }, + // Folder with too many total input streams + buf -> { + final Coder coder = new Coder(new byte[] { 0x03 }, 2, 1, null); + final Coder[] coders = new Coder[33]; + Arrays.fill(coders, coder); + writeFolder(buf, coders); + }, + // Folder with more alternative methods (not supported yet) + buf -> writeFolder(buf, new Coder[]{new Coder(new byte[]{0x03}, 1, 1, null)}, + true, false, false, false), + // Folder with unsupported bind pair in index + buf -> { + final Coder coder = new Coder(new byte[] { 0x03 }, 1, 1, null); + writeFolder(buf, new Coder[] { coder, coder }, false, true, false, false); + }, + // Folder with unsupported bind pair out index + buf -> { + final Coder coder = new Coder(new byte[] { 0x03 }, 1, 1, null); + writeFolder(buf, new Coder[] { coder, coder }, false, false, true, false); + }, + // Folder with unsupported packed stream index + buf -> { + final Coder coder = new Coder(new byte[]{0x03}, 2, 1, null); + writeFolder(buf, new Coder[]{ coder, coder }, false, false, false, true); + } + ); + } + + private static void writeBindPair(final ByteBuffer buffer, final long inIndex, final long outIndex) { + writeUint64(buffer, inIndex); + writeUint64(buffer, outIndex); + } + + private static void writeCoder(final ByteBuffer buffer, final byte[] methodId, final long numInStreams, final long numOutStreams, + final boolean moreAlternativeMethods) { + final boolean isComplex = numInStreams != 1 || numOutStreams != 1; + int flag = methodId.length; + if (isComplex) { + flag |= 0x10; + } + if (moreAlternativeMethods) { + flag |= 0x80; + } + // coder + buffer.put((byte) flag); + buffer.put(methodId); + if (isComplex) { + writeUint64(buffer, numInStreams); + writeUint64(buffer, numOutStreams); + } + } + + private static void writeFolder(final ByteBuffer buffer, final Coder[] coders) { + writeFolder(buffer, coders, false, false, false, false); + } + + private static void writeFolder(final ByteBuffer buffer, final Coder[] coders, final boolean moreAlternativeMethods, final boolean unsupportedBindPairIn, + final boolean unsupportedBindPairOut, final boolean unsupportedPackedStreams) { + writeUint64(buffer, coders.length); + long totalInStreams = 0; + long totalOutStreams = 0; + for (final Coder coder : coders) { + writeCoder(buffer, coder.decompressionMethodId, coder.numInStreams, coder.numOutStreams, moreAlternativeMethods); + totalInStreams += coder.numInStreams; + totalOutStreams += coder.numOutStreams; + } + long i = 0; + // Bind pairs: one less than number of total out streams + for (; i < totalOutStreams - 1; i++) { + final long inIndex = (unsupportedBindPairIn ? totalInStreams : 0) + i; + final long outIndex = (unsupportedBindPairOut ? totalOutStreams : 0) + i + 1; + writeBindPair(buffer, inIndex, outIndex); + } + // Packed streams: one per in stream that is not bound + if (totalInStreams > i + 1) { + for (; i < totalInStreams; i++) { + final long packedStreamIndex = (unsupportedPackedStreams ? totalInStreams : 0) + i; + writeUint64(buffer, packedStreamIndex); + } + } + } + + private static void writeUint64(final ByteBuffer buffer, final long value) { + buffer.put((byte) 0b1111_1111); + buffer.putLong(value); + } + private void assertDate(final SevenZArchiveEntry entry, final String value, final Function hasValue, final Function timeFunction, final Function dateFunction) { if (value != null) { @@ -838,6 +952,21 @@ void testReadEntriesOfSize0() throws IOException { } } + @ParameterizedTest + @MethodSource + void testReadFolder_Unsupported(final Consumer folderWriter) throws IOException { + try (SevenZFile file = SevenZFile.builder().setURI(getURI("bla.7z")).get()) { + // Allocate a buffer large enough to hold the folder data + final ByteBuffer buffer = ByteBuffer.allocate(8192).order(ByteOrder.LITTLE_ENDIAN); + folderWriter.accept(buffer); + buffer.flip(); + final ArchiveException e = assertThrows(ArchiveException.class, () -> { + file.readFolder(buffer); + }); + assertTrue(e.getMessage().contains("Unsupported 7z archive")); + } + } + /** * Test case for COMPRESS-681. */ diff --git a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFolderTest.java b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFolderTest.java index 72139550518..ed2755386b3 100644 --- a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFolderTest.java +++ b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFolderTest.java @@ -53,12 +53,12 @@ void testGetUnpackSizeForCoderOne() { @Test void testGetUnpackSizeOne() throws ArchiveException { final Folder folder = new Folder(); - folder.totalOutputStreams = 266L; + folder.totalOutputStreams = 266; final BindPair[] bindPairArray = new BindPair[1]; final BindPair bindPair = new BindPair(0, 0); bindPairArray[0] = bindPair; folder.bindPairs = bindPairArray; - folder.totalOutputStreams = 1L; + folder.totalOutputStreams = 1; assertEquals(0L, folder.getUnpackSize()); }