You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by bo...@apache.org on 2021/05/13 17:10:11 UTC

[commons-compress] branch master updated: COMPRESS-542 sanity check 7z metadata with minimizing allocations

This is an automated email from the ASF dual-hosted git repository.

bodewig 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 26924e9  COMPRESS-542 sanity check 7z metadata with minimizing allocations
26924e9 is described below

commit 26924e96c7730db014c310757e11c9359db07f3e
Author: Stefan Bodewig <st...@innoq.com>
AuthorDate: Thu May 13 19:08:43 2021 +0200

    COMPRESS-542 sanity check 7z metadata with minimizing allocations
---
 .../compress/archivers/sevenz/SevenZFile.java      | 453 ++++++++++++++++++++-
 1 file changed, 452 insertions(+), 1 deletion(-)

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 2f24547..9f5986f 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
@@ -550,13 +550,29 @@ public class SevenZFile implements Closeable {
         try (DataInputStream dataInputStream = new DataInputStream(new CRC32VerifyingInputStream(
                 new BoundedSeekableByteChannelInputStream(channel, 20), 20, startHeaderCrc))) {
              startHeader.nextHeaderOffset = Long.reverseBytes(dataInputStream.readLong());
+             if (startHeader.nextHeaderOffset < 0
+                 || startHeader.nextHeaderOffset + SIGNATURE_HEADER_SIZE > channel.size()) {
+                 throw new IOException("nextHeaderOffset is out of bounds");
+             }
+
              startHeader.nextHeaderSize = Long.reverseBytes(dataInputStream.readLong());
+             final long nextHeaderEnd = startHeader.nextHeaderOffset + startHeader.nextHeaderSize;
+             if (nextHeaderEnd < startHeader.nextHeaderOffset
+                 || nextHeaderEnd + SIGNATURE_HEADER_SIZE > channel.size()) {
+                 throw new IOException("nextHeaderSize is out of bounds");
+             }
+
              startHeader.nextHeaderCrc = 0xffffFFFFL & Integer.reverseBytes(dataInputStream.readInt());
+
              return startHeader;
         }
     }
 
     private void readHeader(final ByteBuffer header, final Archive archive) throws IOException {
+        final int pos = header.position();
+        final ArchiveStatistics stats = sanityCheckAndCollectStatistics(header);
+        header.position(pos);
+
         int nid = getUnsignedByte(header);
 
         if (nid == NID.kArchiveProperties) {
@@ -584,6 +600,39 @@ public class SevenZFile implements Closeable {
         }
     }
 
+    private ArchiveStatistics sanityCheckAndCollectStatistics(final ByteBuffer header)
+        throws IOException {
+        final ArchiveStatistics stats = new ArchiveStatistics();
+
+        int nid = getUnsignedByte(header);
+
+        if (nid == NID.kArchiveProperties) {
+            sanityCheckArchiveProperties(header, stats);
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid == NID.kAdditionalStreamsInfo) {
+            throw new IOException("Additional streams unsupported");
+            //nid = header.readUnsignedByte();
+        }
+
+        if (nid == NID.kMainStreamsInfo) {
+            sanityCheckStreamsInfo(header, stats);
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid == NID.kFilesInfo) {
+            sanityCheckFilesInfo(header, stats);
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid != NID.kEnd) {
+            throw new IOException("Badly terminated header, found " + nid);
+        }
+
+        return stats;
+    }
+
     private void readArchiveProperties(final ByteBuffer input) throws IOException {
         // FIXME: the reference implementation just throws them away?
         int nid =  getUnsignedByte(input);
@@ -596,8 +645,24 @@ public class SevenZFile implements Closeable {
         }
     }
 
+    private void sanityCheckArchiveProperties(final ByteBuffer header, final ArchiveStatistics stats)
+        throws IOException {
+        int nid =  getUnsignedByte(header);
+        while (nid != NID.kEnd) {
+            final int propertySize =
+                assertFitsIntoNonNegativeInt("propertySize", readUint64(header));
+            if (skipBytesFully(header, propertySize) < propertySize) {
+                throw new IOException("invalid property size");
+            }
+            nid = getUnsignedByte(header);
+        }
+    }
+
     private ByteBuffer readEncodedHeader(final ByteBuffer header, final Archive archive,
                                          final byte[] password) throws IOException {
+        final int pos = header.position();
+        sanityCheckStreamsInfo(header, new ArchiveStatistics());
+        header.position(pos);
         readStreamsInfo(header, archive);
 
         // FIXME: merge with buildDecodingStream()/buildDecoderStack() at some stage?
@@ -628,6 +693,30 @@ public class SevenZFile implements Closeable {
         return ByteBuffer.wrap(nextHeader).order(ByteOrder.LITTLE_ENDIAN);
     }
 
+    private void sanityCheckStreamsInfo(final ByteBuffer header,
+        final ArchiveStatistics stats) throws IOException {
+        int nid = getUnsignedByte(header);
+
+        if (nid == NID.kPackInfo) {
+            sanityCheckPackInfo(header, stats);
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid == NID.kUnpackInfo) {
+            sanityCheckUnpackInfo(header, stats);
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid == NID.kSubStreamsInfo) {
+            sanityCheckSubStreamsInfo(header, stats);
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid != NID.kEnd) {
+            throw new IOException("Badly terminated StreamsInfo");
+        }
+    }
+
     private void readStreamsInfo(final ByteBuffer header, final Archive archive) throws IOException {
         int nid = getUnsignedByte(header);
 
@@ -654,6 +743,45 @@ public class SevenZFile implements Closeable {
         }
     }
 
+    private void sanityCheckPackInfo(final ByteBuffer header, final ArchiveStatistics stats) throws IOException {
+        final long packPos = readUint64(header);
+        if (packPos < 0 || SIGNATURE_HEADER_SIZE + packPos > channel.size()
+            || SIGNATURE_HEADER_SIZE + packPos < 0) {
+            throw new IOException("packPos (" + packPos + ") is out of range");
+        }
+        stats.packPos = packPos;
+        final long numPackStreams = readUint64(header);
+        stats.numberOfPackedStreams = assertFitsIntoNonNegativeInt("numPackStreams", numPackStreams);
+        int nid = getUnsignedByte(header);
+        if (nid == NID.kSize) {
+            long totalPackSizes = 0;
+            for (int i = 0; i < stats.numberOfPackedStreams; i++) {
+                final long packSize = readUint64(header);
+                totalPackSizes += packSize;
+                final long endOfPackStreams = SIGNATURE_HEADER_SIZE + packPos + totalPackSizes;
+                if (packSize < 0
+                    || endOfPackStreams > channel.size()
+                    || endOfPackStreams < packPos) {
+                    throw new IOException("packSize (" + packSize + ") is out of range");
+                }
+            }
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid == NID.kCRC) {
+            final int crcsDefined = readAllOrBits(header, stats.numberOfPackedStreams)
+                .cardinality();
+            if (skipBytesFully(header, 4 * crcsDefined) < 4 * crcsDefined) {
+                throw new IOException("invalid number of CRCs in PackInfo");
+            }
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid != NID.kEnd) {
+            throw new IOException("Badly terminated PackInfo (" + nid + ")");
+        }
+    }
+
     private void readPackInfo(final ByteBuffer header, final Archive archive) throws IOException {
         archive.packPos = readUint64(header);
         final long numPackStreams = readUint64(header);
@@ -685,6 +813,53 @@ public class SevenZFile implements Closeable {
         }
     }
 
+    private void sanityCheckUnpackInfo(final ByteBuffer header, final ArchiveStatistics stats)
+        throws IOException {
+        int nid = getUnsignedByte(header);
+        if (nid != NID.kFolder) {
+            throw new IOException("Expected kFolder, got " + nid);
+        }
+        final long numFolders = readUint64(header);
+        stats.numberOfFolders = assertFitsIntoNonNegativeInt("numFolders", numFolders);
+        final int external = getUnsignedByte(header);
+        if (external != 0) {
+            throw new IOException("External unsupported");
+        }
+
+        final List<Integer> numberOfOutputStreamsPerFolder = new LinkedList<>();
+        for (int i = 0; i < stats.numberOfFolders; i++) {
+            numberOfOutputStreamsPerFolder.add(sanityCheckFolder(header, stats));
+        }
+
+        nid = getUnsignedByte(header);
+        if (nid != NID.kCodersUnpackSize) {
+            throw new IOException("Expected kCodersUnpackSize, got " + nid);
+        }
+
+        for (int numberOfOutputStreams : numberOfOutputStreamsPerFolder) {
+            for (int i = 0; i < numberOfOutputStreams; i++) {
+                final long unpackSize = readUint64(header);
+                if (unpackSize < 0) {
+                    throw new IllegalArgumentException("negative unpackSize");
+                }
+            }
+        }
+
+        nid = getUnsignedByte(header);
+        if (nid == NID.kCRC) {
+            stats.folderHasCrc = readAllOrBits(header, stats.numberOfFolders);
+            final int crcsDefined = stats.folderHasCrc.cardinality();
+            if (skipBytesFully(header, 4 * crcsDefined) < 4 * crcsDefined) {
+                throw new IOException("invalid number of CRCs in UnpackInfo");
+            }
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid != NID.kEnd) {
+            throw new IOException("Badly terminated UnpackInfo");
+        }
+    }
+
     private void readUnpackInfo(final ByteBuffer header, final Archive archive) throws IOException {
         int nid = getUnsignedByte(header);
         if (nid != NID.kFolder) {
@@ -735,6 +910,64 @@ public class SevenZFile implements Closeable {
         }
     }
 
+    private void sanityCheckSubStreamsInfo(final ByteBuffer header, final ArchiveStatistics stats) throws IOException {
+
+        int nid = getUnsignedByte(header);
+        final List<Integer> numUnpackSubStreamsPerFolder = new LinkedList<>();
+        if (nid == NID.kNumUnpackStream) {
+            for (int i = 0; i < stats.numberOfFolders; i++) {
+                numUnpackSubStreamsPerFolder.add(assertFitsIntoNonNegativeInt("numStreams", readUint64(header)));
+            }
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid == NID.kSize) {
+            for (final int numUnpackSubStreams : numUnpackSubStreamsPerFolder) {
+                if (numUnpackSubStreams == 0) {
+                    continue;
+                }
+                long sum = 0;
+                for (int i = 0; i < numUnpackSubStreams - 1; i++) {
+                    final long size = readUint64(header);
+                    if (size < 0) {
+                        throw new IOException("negative unpackSize");
+                    }
+                    sum += size;
+                }
+                // TODO sum < folder.unpackSize
+            }
+            nid = getUnsignedByte(header);
+        }
+
+        int numDigests = 0;
+        if (numUnpackSubStreamsPerFolder.isEmpty()) {
+            numDigests = stats.folderHasCrc == null ? stats.numberOfFolders
+                : stats.numberOfFolders - stats.folderHasCrc.cardinality();
+        } else {
+            int folderIdx = 0;
+            for (final int numUnpackSubStreams : numUnpackSubStreamsPerFolder) {
+                if (numUnpackSubStreams != 1 || stats.folderHasCrc == null
+                    || !stats.folderHasCrc.get(folderIdx++)) {
+                    numDigests += numUnpackSubStreams;
+                }
+            }
+        }
+
+        if (nid == NID.kCRC) {
+            assertFitsIntoNonNegativeInt("numDigests", numDigests);
+            final int missingCrcs = readAllOrBits(header, numDigests)
+                .cardinality();
+            if (skipBytesFully(header, 4 * missingCrcs) < 4 * missingCrcs) {
+                throw new IOException("invalid number of missing CRCs in SubStreamInfo");
+            }
+            nid = getUnsignedByte(header);
+        }
+
+        if (nid != NID.kEnd) {
+            throw new IOException("Badly terminated SubStreamsInfo");
+        }
+    }
+
     private void readSubStreamsInfo(final ByteBuffer header, final Archive archive) throws IOException {
         for (final Folder folder : archive.folders) {
             folder.numUnpackSubStreams = 1;
@@ -820,6 +1053,88 @@ public class SevenZFile implements Closeable {
         archive.subStreamsInfo = subStreamsInfo;
     }
 
+    private int sanityCheckFolder(final ByteBuffer header, final ArchiveStatistics stats)
+        throws IOException {
+
+        final int numCoders = assertFitsIntoNonNegativeInt("numCoders", readUint64(header));
+
+        long totalOutStreams = 0;
+        long totalInStreams = 0;
+        for (int i = 0; i < numCoders; i++) {
+            final int bits = getUnsignedByte(header);
+            final int idSize = bits & 0xf;
+            header.get(new byte[idSize]);
+
+            final boolean isSimple = (bits & 0x10) == 0;
+            final boolean hasAttributes = (bits & 0x20) != 0;
+            final boolean moreAlternativeMethods = (bits & 0x80) != 0;
+            if (moreAlternativeMethods) {
+                throw new IOException("Alternative methods are unsupported, please report. " + // NOSONAR
+                    "The reference implementation doesn't support them either.");
+            }
+
+            if (isSimple) {
+                totalInStreams++;
+                totalOutStreams++;
+            } else {
+                totalInStreams +=
+                    assertFitsIntoNonNegativeInt("numInStreams", readUint64(header));
+                totalOutStreams +=
+                    assertFitsIntoNonNegativeInt("numOutStreams", readUint64(header));
+            }
+
+            if (hasAttributes) {
+                final int propertiesSize =
+                    assertFitsIntoNonNegativeInt("propertiesSize", readUint64(header));
+                if (skipBytesFully(header, propertiesSize) < propertiesSize) {
+                    throw new IOException("invalid propertiesSize in folder");
+                }
+            }
+        }
+        assertFitsIntoNonNegativeInt("totalInStreams", totalInStreams);
+        assertFitsIntoNonNegativeInt("totalOutStreams", totalOutStreams);
+
+        if (totalOutStreams == 0) {
+            throw new IOException("Total output streams can't be 0");
+        }
+
+        final int numBindPairs =
+            assertFitsIntoNonNegativeInt("numBindPairs", totalOutStreams - 1);
+        if (totalInStreams < numBindPairs) {
+            throw new IOException("Total input streams can't be less than the number of bind pairs");
+        }
+        final BitSet inStreamsBound = new BitSet((int) totalInStreams);
+        for (int i = 0; i < numBindPairs; i++) {
+            final int inIndex = assertFitsIntoNonNegativeInt("inIndex", readUint64(header));
+            if (totalInStreams <= inIndex) {
+                throw new IOException("inIndex is bigger than number of inStreams");
+            }
+            inStreamsBound.set(inIndex);
+            final int outIndex = assertFitsIntoNonNegativeInt("outIndex", readUint64(header));
+            if (totalOutStreams <= outIndex) {
+                throw new IOException("outIndex is bigger than number of outStreams");
+            }
+        }
+
+        final int numPackedStreams =
+            assertFitsIntoNonNegativeInt("numPackedStreams", totalInStreams - numBindPairs);
+        if (numPackedStreams == 1) {
+            if (inStreamsBound.nextClearBit(0) == -1) {
+                throw new IOException("Couldn't find stream's bind pair index");
+            }
+        } else {
+            for (int i = 0; i < numPackedStreams; i++) {
+                final int packedStreamIndex =
+                    assertFitsIntoNonNegativeInt("packedStreamIndex", readUint64(header));
+                if (packedStreamIndex >= stats.numberOfPackedStreams) {
+                    throw new IOException("packedStreamIndex is bigger than number of packed streams");
+                }
+            }
+        }
+
+        return (int) totalOutStreams;
+    }
+
     private Folder readFolder(final ByteBuffer header) throws IOException {
         final Folder folder = new Folder();
 
@@ -934,6 +1249,133 @@ public class SevenZFile implements Closeable {
         return bits;
     }
 
+    private void sanityCheckFilesInfo(final ByteBuffer header, final ArchiveStatistics stats) throws IOException {
+        stats.numberOfEntries = assertFitsIntoNonNegativeInt("numFiles", readUint64(header));
+
+        int emptyStreams = -1;
+        int emptyFiles = -1;
+        int antis = -1;
+        while (true) {
+            final int propertyType = getUnsignedByte(header);
+            if (propertyType == 0) {
+                break;
+            }
+            final long size = readUint64(header);
+            switch (propertyType) {
+                case NID.kEmptyStream: {
+                    emptyStreams = readBits(header, stats.numberOfEntries).cardinality();
+                    break;
+                }
+                case NID.kEmptyFile: {
+                    if (emptyStreams == -1) {
+                        throw new IOException("Header format error: kEmptyStream must appear before kEmptyFile");
+                    }
+                    emptyFiles = readBits(header, emptyStreams).cardinality();
+                    break;
+                }
+                case NID.kAnti: {
+                    if (emptyStreams == -1) {
+                        throw new IOException("Header format error: kEmptyStream must appear before kAnti");
+                    }
+                    antis = readBits(header, emptyStreams).cardinality();
+                    break;
+                }
+                case NID.kName: {
+                    final int external = getUnsignedByte(header);
+                    if (external != 0) {
+                        throw new IOException("Not implemented");
+                    }
+                    if (((size - 1) & 1) != 0) {
+                        throw new IOException("File names length invalid");
+                    }
+                    final int namesLength =
+                        assertFitsIntoNonNegativeInt("file names length", size - 1);
+
+                    int filesSeen = 0;
+                    for (int i = 0; i < namesLength; i += 2) {
+                        final char c = header.getChar();
+                        if (c == 0) {
+                            filesSeen++;
+                        }
+                    }
+                    if (filesSeen != stats.numberOfEntries) {
+                        throw new IOException("Invalid number of file names (" + filesSeen + " instead of "
+                            + stats.numberOfEntries + ")");
+                    }
+                    break;
+                }
+                case NID.kCTime: {
+                    final int timesDefined = readAllOrBits(header, stats.numberOfEntries)
+                        .cardinality();
+                    final int external = getUnsignedByte(header);
+                    if (external != 0) {
+                        throw new IOException("Not implemented");
+                    }
+                    if (skipBytesFully(header, 8 * timesDefined) < 8 * timesDefined) {
+                        throw new IOException("invalid creation dates size");
+                    }
+                    break;
+                }
+                case NID.kATime: {
+                    final int timesDefined = readAllOrBits(header, stats.numberOfEntries)
+                        .cardinality();
+                    final int external = getUnsignedByte(header);
+                    if (external != 0) {
+                        throw new IOException("Not implemented");
+                    }
+                    if (skipBytesFully(header, 8 * timesDefined) < 8 * timesDefined) {
+                        throw new IOException("invalid access dates size");
+                    }
+                    break;
+                }
+                case NID.kMTime: {
+                    final int timesDefined = readAllOrBits(header, stats.numberOfEntries)
+                        .cardinality();
+                    final int external = getUnsignedByte(header);
+                    if (external != 0) {
+                        throw new IOException("Not implemented");
+                    }
+                    if (skipBytesFully(header, 8 * timesDefined) < 8 * timesDefined) {
+                        throw new IOException("invalid modification dates size");
+                    }
+                    break;
+                }
+                case NID.kWinAttributes: {
+                    final int attributesDefined = readAllOrBits(header, stats.numberOfEntries)
+                        .cardinality();
+                    final int external = getUnsignedByte(header);
+                    if (external != 0) {
+                        throw new IOException("Not implemented");
+                    }
+                    if (skipBytesFully(header, 4 * attributesDefined) < 4 * attributesDefined) {
+                        throw new IOException("invalid windows attributes size");
+                    }
+                    break;
+                }
+                case NID.kStartPos: {
+                    throw new IOException("kStartPos is unsupported, please report");
+                }
+                case NID.kDummy: {
+                    // 7z 9.20 asserts the content is all zeros and ignores the property
+                    // Compress up to 1.8.1 would throw an exception, now we ignore it (see COMPRESS-287
+
+                    if (skipBytesFully(header, size) < size) {
+                        throw new IOException("Incomplete kDummy property");
+                    }
+                    break;
+                }
+
+                default: {
+                    // Compress up to 1.8.1 would throw an exception, now we ignore it (see COMPRESS-287
+                    if (skipBytesFully(header, size) < size) {
+                        throw new IOException("Incomplete property of type " + propertyType);
+                    }
+                    break;
+                }
+            }
+        }
+    }
+
     private void readFilesInfo(final ByteBuffer header, final Archive archive) throws IOException {
         final long numFiles = readUint64(header);
         assertFitsIntoNonNegativeInt("numFiles", numFiles);
@@ -1643,9 +2085,18 @@ public class SevenZFile implements Closeable {
         return e;
     }
 
-    private static void assertFitsIntoNonNegativeInt(final String what, final long value) throws IOException {
+    private static int assertFitsIntoNonNegativeInt(final String what, final long value) throws IOException {
         if (value > Integer.MAX_VALUE || value < 0) {
             throw new IOException("Cannot handle " + what + " " + value);
         }
+        return (int) value;
+    }
+
+    private static class ArchiveStatistics {
+        private long packPos;
+        private int numberOfPackedStreams;
+        private int numberOfFolders;
+        private BitSet folderHasCrc;
+        private int numberOfEntries;
     }
 }