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 2a504575c8..aacf34a544 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 @@ -37,7 +37,10 @@ import java.util.Arrays; import java.util.BitSet; import java.util.EnumSet; +import java.util.HashMap; import java.util.LinkedList; +import java.util.List; +import java.util.Map; import java.util.zip.CRC32; import org.apache.commons.compress.utils.BoundedInputStream; @@ -934,10 +937,8 @@ private BitSet readBits(final ByteBuffer header, final int size) throws IOExcept private void readFilesInfo(final ByteBuffer header, final Archive archive) throws IOException { final long numFiles = readUint64(header); assertFitsIntoInt("numFiles", numFiles); - final SevenZArchiveEntry[] files = new SevenZArchiveEntry[(int)numFiles]; - for (int i = 0; i < files.length; i++) { - files[i] = new SevenZArchiveEntry(); - } + final int numFilesInt = (int) numFiles; + final Map fileMap = new HashMap<>(); BitSet isEmptyStream = null; BitSet isEmptyFile = null; BitSet isAnti = null; @@ -949,7 +950,7 @@ private void readFilesInfo(final ByteBuffer header, final Archive archive) throw final long size = readUint64(header); switch (propertyType) { case NID.kEmptyStream: { - isEmptyStream = readBits(header, files.length); + isEmptyStream = readBits(header, numFilesInt); break; } case NID.kEmptyFile: { @@ -980,68 +981,78 @@ private void readFilesInfo(final ByteBuffer header, final Archive archive) throw int nextFile = 0; int nextName = 0; for (int i = 0; i < names.length; i += 2) { - if (names[i] == 0 && names[i+1] == 0) { - files[nextFile++].setName(new String(names, nextName, i-nextName, StandardCharsets.UTF_16LE)); + if (names[i] == 0 && names[i + 1] == 0) { + checkEntryIsInitialized(fileMap, nextFile); + fileMap.get(nextFile).setName(new String(names, nextName, i - nextName, StandardCharsets.UTF_16LE)); nextName = i + 2; + nextFile++; } } - if (nextName != names.length || nextFile != files.length) { + if (nextName != names.length || nextFile != numFiles) { throw new IOException("Error parsing file names"); } break; } case NID.kCTime: { - final BitSet timesDefined = readAllOrBits(header, files.length); + final BitSet timesDefined = readAllOrBits(header, numFilesInt); final int external = getUnsignedByte(header); if (external != 0) { throw new IOException("Unimplemented"); } - for (int i = 0; i < files.length; i++) { - files[i].setHasCreationDate(timesDefined.get(i)); - if (files[i].getHasCreationDate()) { - files[i].setCreationDate(header.getLong()); + for (int i = 0; i < numFilesInt; i++) { + checkEntryIsInitialized(fileMap, i); + final SevenZArchiveEntry entryAtIndex = fileMap.get(i); + entryAtIndex.setHasCreationDate(timesDefined.get(i)); + if (entryAtIndex.getHasCreationDate()) { + entryAtIndex.setCreationDate(header.getLong()); } } break; } case NID.kATime: { - final BitSet timesDefined = readAllOrBits(header, files.length); + final BitSet timesDefined = readAllOrBits(header, numFilesInt); final int external = getUnsignedByte(header); if (external != 0) { throw new IOException("Unimplemented"); } - for (int i = 0; i < files.length; i++) { - files[i].setHasAccessDate(timesDefined.get(i)); - if (files[i].getHasAccessDate()) { - files[i].setAccessDate(header.getLong()); + for (int i = 0; i < numFilesInt; i++) { + checkEntryIsInitialized(fileMap, i); + final SevenZArchiveEntry entryAtIndex = fileMap.get(i); + entryAtIndex.setHasAccessDate(timesDefined.get(i)); + if (entryAtIndex.getHasAccessDate()) { + entryAtIndex.setAccessDate(header.getLong()); } } break; } case NID.kMTime: { - final BitSet timesDefined = readAllOrBits(header, files.length); + final BitSet timesDefined = readAllOrBits(header, numFilesInt); final int external = getUnsignedByte(header); if (external != 0) { throw new IOException("Unimplemented"); } - for (int i = 0; i < files.length; i++) { - files[i].setHasLastModifiedDate(timesDefined.get(i)); - if (files[i].getHasLastModifiedDate()) { - files[i].setLastModifiedDate(header.getLong()); + for (int i = 0; i < numFilesInt; i++) { + checkEntryIsInitialized(fileMap, i); + final SevenZArchiveEntry entryAtIndex = fileMap.get(i); + entryAtIndex.setHasLastModifiedDate(timesDefined.get(i)); + if (entryAtIndex.getHasLastModifiedDate()) { + entryAtIndex.setLastModifiedDate(header.getLong()); } } break; } case NID.kWinAttributes: { - final BitSet attributesDefined = readAllOrBits(header, files.length); + final BitSet attributesDefined = readAllOrBits(header, numFilesInt); final int external = getUnsignedByte(header); if (external != 0) { throw new IOException("Unimplemented"); } - for (int i = 0; i < files.length; i++) { - files[i].setHasWindowsAttributes(attributesDefined.get(i)); - if (files[i].getHasWindowsAttributes()) { - files[i].setWindowsAttributes(header.getInt()); + for (int i = 0; i < numFilesInt; i++) { + checkEntryIsInitialized(fileMap, i); + final SevenZArchiveEntry entryAtIndex = fileMap.get(i); + entryAtIndex.setHasWindowsAttributes(attributesDefined.get(i)); + if (entryAtIndex.getHasWindowsAttributes()) { + entryAtIndex.setWindowsAttributes(header.getInt()); } } break; @@ -1070,30 +1081,46 @@ private void readFilesInfo(final ByteBuffer header, final Archive archive) throw } int nonEmptyFileCounter = 0; int emptyFileCounter = 0; - for (int i = 0; i < files.length; i++) { - files[i].setHasStream(isEmptyStream == null || !isEmptyStream.get(i)); - if (files[i].hasStream()) { + for (int i = 0; i < numFilesInt; i++) { + final SevenZArchiveEntry entryAtIndex = fileMap.get(i); + if (entryAtIndex == null) { + continue; + } + entryAtIndex.setHasStream(isEmptyStream == null || !isEmptyStream.get(i)); + if (entryAtIndex.hasStream()) { if (archive.subStreamsInfo == null) { throw new IOException("Archive contains file with streams but no subStreamsInfo"); } - files[i].setDirectory(false); - files[i].setAntiItem(false); - files[i].setHasCrc(archive.subStreamsInfo.hasCrc.get(nonEmptyFileCounter)); - files[i].setCrcValue(archive.subStreamsInfo.crcs[nonEmptyFileCounter]); - files[i].setSize(archive.subStreamsInfo.unpackSizes[nonEmptyFileCounter]); + entryAtIndex.setDirectory(false); + entryAtIndex.setAntiItem(false); + entryAtIndex.setHasCrc(archive.subStreamsInfo.hasCrc.get(nonEmptyFileCounter)); + entryAtIndex.setCrcValue(archive.subStreamsInfo.crcs[nonEmptyFileCounter]); + entryAtIndex.setSize(archive.subStreamsInfo.unpackSizes[nonEmptyFileCounter]); ++nonEmptyFileCounter; } else { - files[i].setDirectory(isEmptyFile == null || !isEmptyFile.get(emptyFileCounter)); - files[i].setAntiItem(isAnti != null && isAnti.get(emptyFileCounter)); - files[i].setHasCrc(false); - files[i].setSize(0); + entryAtIndex.setDirectory(isEmptyFile == null || !isEmptyFile.get(emptyFileCounter)); + entryAtIndex.setAntiItem(isAnti != null && isAnti.get(emptyFileCounter)); + entryAtIndex.setHasCrc(false); + entryAtIndex.setSize(0); ++emptyFileCounter; } } - archive.files = files; + final List entries = new ArrayList<>(); + for (final SevenZArchiveEntry e : fileMap.values()) { + if (e != null) { + entries.add(e); + } + } + archive.files = entries.toArray(new SevenZArchiveEntry[0]); calculateStreamMap(archive); } + private void checkEntryIsInitialized(final Map archiveEntries, final int index) { + if (archiveEntries.get(index) == null) { + archiveEntries.put(index, new SevenZArchiveEntry()); + } + } + private void calculateStreamMap(final Archive archive) throws IOException { final StreamMap streamMap = new StreamMap(); 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 29f959997c..2e91822ed4 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 @@ -28,9 +28,11 @@ import java.nio.file.Files; import java.nio.file.Path; import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Random; @@ -708,6 +710,23 @@ public void retrieveInputStreamForAllEntriesWithoutCRCMultipleTimes() throws IOE } } + @Test + public void testNoOOMOnCorruptedHeader() throws IOException { + final List testFiles = new ArrayList<>(); + testFiles.add(getPath("COMPRESS-542-1.7z")); + testFiles.add(getPath("COMPRESS-542-2.7z")); + testFiles.add(getPath("COMPRESS-542-endheadercorrupted.7z")); + testFiles.add(getPath("COMPRESS-542-endheadercorrupted2.7z")); + + for (final Path file : testFiles) { + try (SevenZFile sevenZFile = new SevenZFile(Files.newByteChannel(file))) { + fail("Expected IOException: start header corrupt and unable to guess end header"); + } catch (IOException e) { + assertEquals("Start header corrupt and unable to guess end header", e.getMessage()); + } + } + } + private void test7zUnarchive(final File f, final SevenZMethod m, final byte[] password) throws Exception { try (SevenZFile sevenZFile = new SevenZFile(f, password)) { test7zUnarchive(sevenZFile, m); diff --git a/src/test/resources/COMPRESS-542-1.7z b/src/test/resources/COMPRESS-542-1.7z new file mode 100644 index 0000000000..b5bd519cd0 Binary files /dev/null and b/src/test/resources/COMPRESS-542-1.7z differ diff --git a/src/test/resources/COMPRESS-542-2.7z b/src/test/resources/COMPRESS-542-2.7z new file mode 100644 index 0000000000..8c68e75baf Binary files /dev/null and b/src/test/resources/COMPRESS-542-2.7z differ diff --git a/src/test/resources/COMPRESS-542-endheadercorrupted.7z b/src/test/resources/COMPRESS-542-endheadercorrupted.7z new file mode 100644 index 0000000000..93b3e9d019 Binary files /dev/null and b/src/test/resources/COMPRESS-542-endheadercorrupted.7z differ diff --git a/src/test/resources/COMPRESS-542-endheadercorrupted2.7z b/src/test/resources/COMPRESS-542-endheadercorrupted2.7z new file mode 100644 index 0000000000..f21f7c416a Binary files /dev/null and b/src/test/resources/COMPRESS-542-endheadercorrupted2.7z differ