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/06/11 21:10:32 UTC
[commons-compress] branch master updated (e1e5635 -> 4eea958)
This is an automated email from the ASF dual-hosted git repository.
bodewig pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git.
from e1e5635 Bump actions/cache from 2.1.4 to 2.1.6 (#200)
new 3af47ee simplify comparator
new 4eea958 ZipFile could use some sanity checks as well
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../commons/compress/archivers/zip/ZipFile.java | 84 ++++++++++++++--------
.../commons/compress/archivers/ZipTestCase.java | 4 +-
2 files changed, 56 insertions(+), 32 deletions(-)
[commons-compress] 01/02: simplify comparator
Posted by bo...@apache.org.
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
commit 3af47ee06aab8fd715c304695d18ebc88eb9aa09
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Fri Jun 11 22:41:19 2021 +0200
simplify comparator
---
.../commons/compress/archivers/zip/ZipFile.java | 25 ++--------------------
1 file changed, 2 insertions(+), 23 deletions(-)
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java
index 8792563..3ae2ecf 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java
@@ -1390,29 +1390,8 @@ public class ZipFile implements Closeable {
* @since 1.1
*/
private final Comparator<ZipArchiveEntry> offsetComparator =
- (e1, e2) -> {
- if (e1 == e2) {
- return 0;
- }
-
- final Entry ent1 = e1 instanceof Entry ? (Entry) e1 : null;
- final Entry ent2 = e2 instanceof Entry ? (Entry) e2 : null;
- if (ent1 == null) {
- return 1;
- }
- if (ent2 == null) {
- return -1;
- }
-
- // disk number is prior to relative offset
- final long diskNumberStartVal = ent1.getDiskNumberStart() - ent2.getDiskNumberStart();
- if (diskNumberStartVal != 0) {
- return diskNumberStartVal < 0 ? -1 : +1;
- }
- final long val = (ent1.getLocalHeaderOffset()
- - ent2.getLocalHeaderOffset());
- return val == 0 ? 0 : val < 0 ? -1 : +1;
- };
+ Comparator.comparingLong(ZipArchiveEntry::getDiskNumberStart)
+ .thenComparingLong(ZipArchiveEntry::getLocalHeaderOffset);
/**
* Extends ZipArchiveEntry to store the offset within the archive.
[commons-compress] 02/02: ZipFile could use some sanity checks as
well
Posted by bo...@apache.org.
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
commit 4eea95853e15e832104118b392a6a1cd0ce05841
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Fri Jun 11 23:08:42 2021 +0200
ZipFile could use some sanity checks as well
---
.../commons/compress/archivers/zip/ZipFile.java | 59 +++++++++++++++++++---
.../commons/compress/archivers/ZipTestCase.java | 4 +-
2 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java
index 3ae2ecf..0f2bb53 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java
@@ -160,6 +160,9 @@ public class ZipFile implements Closeable {
private final ByteBuffer cfhBbuf = ByteBuffer.wrap(cfhBuf);
private final ByteBuffer shortBbuf = ByteBuffer.wrap(shortBuf);
+ private long centralDirectoryStartDiskNumber, centralDirectoryStartRelativeOffset;
+ private long centralDirectoryStartOffset;
+
/**
* Opens the given file for reading, assuming "UTF8" for file names.
*
@@ -708,6 +711,7 @@ public class ZipFile implements Closeable {
new HashMap<>();
positionAtCentralDirectory();
+ centralDirectoryStartOffset = archive.position();
wordBbuf.rewind();
IOUtils.readFully(archive, wordBbuf);
@@ -791,12 +795,21 @@ public class ZipFile implements Closeable {
final int fileNameLen = ZipShort.getValue(cfhBuf, off);
off += SHORT;
+ if (fileNameLen < 0) {
+ throw new IOException("broken archive, entry with negative fileNameLen");
+ }
final int extraLen = ZipShort.getValue(cfhBuf, off);
off += SHORT;
+ if (extraLen < 0) {
+ throw new IOException("broken archive, entry with negative extraLen");
+ }
final int commentLen = ZipShort.getValue(cfhBuf, off);
off += SHORT;
+ if (commentLen < 0) {
+ throw new IOException("broken archive, entry with negative commentLen");
+ }
ze.setDiskNumberStart(ZipShort.getValue(cfhBuf, off));
off += SHORT;
@@ -827,6 +840,7 @@ public class ZipFile implements Closeable {
}
setSizesAndOffsetFromZip64Extra(ze);
+ sanityCheckLFHOffset(ze);
final byte[] comment = new byte[commentLen];
IOUtils.readFully(archive, ByteBuffer.wrap(comment));
@@ -839,6 +853,28 @@ public class ZipFile implements Closeable {
ze.setStreamContiguous(true);
}
+ private void sanityCheckLFHOffset(final ZipArchiveEntry ze) throws IOException {
+ if (ze.getDiskNumberStart() < 0) {
+ throw new IOException("broken archive, entry with negative disk number");
+ }
+ if (ze.getLocalHeaderOffset() < 0) {
+ throw new IOException("broken archive, entry with negative local file header offset");
+ }
+ if (isSplitZipArchive) {
+ if (ze.getDiskNumberStart() > centralDirectoryStartDiskNumber) {
+ throw new IOException("local file header for " + ze.getName() + " starts on a later disk than central directory");
+ }
+ if (ze.getDiskNumberStart() == centralDirectoryStartDiskNumber
+ && ze.getLocalHeaderOffset() > centralDirectoryStartRelativeOffset) {
+ throw new IOException("local file header for " + ze.getName() + " starts after central directory");
+ }
+ } else {
+ if (ze.getLocalHeaderOffset() > centralDirectoryStartOffset) {
+ throw new IOException("local file header for " + ze.getName() + " starts after central directory");
+ }
+ }
+ }
+
/**
* If the entry holds a Zip64 extended information extra field,
* read sizes from there if the entry's sizes are set to
@@ -1115,21 +1151,23 @@ public class ZipFile implements Closeable {
- WORD /* signature has already been read */);
wordBbuf.rewind();
IOUtils.readFully(archive, wordBbuf);
- final long diskNumberOfCFD = ZipLong.getValue(wordBuf);
+ centralDirectoryStartDiskNumber = ZipLong.getValue(wordBuf);
skipBytes(ZIP64_EOCD_CFD_LOCATOR_RELATIVE_OFFSET);
dwordBbuf.rewind();
IOUtils.readFully(archive, dwordBbuf);
- final long relativeOffsetOfCFD = ZipEightByteInteger.getLongValue(dwordBuf);
+ centralDirectoryStartRelativeOffset = ZipEightByteInteger.getLongValue(dwordBuf);
((ZipSplitReadOnlySeekableByteChannel) archive)
- .position(diskNumberOfCFD, relativeOffsetOfCFD);
+ .position(centralDirectoryStartDiskNumber, centralDirectoryStartRelativeOffset);
} else {
skipBytes(ZIP64_EOCD_CFD_LOCATOR_OFFSET
- WORD /* signature has already been read */);
dwordBbuf.rewind();
IOUtils.readFully(archive, dwordBbuf);
- archive.position(ZipEightByteInteger.getLongValue(dwordBuf));
+ centralDirectoryStartDiskNumber = 0;
+ centralDirectoryStartRelativeOffset = ZipEightByteInteger.getLongValue(dwordBuf);
+ archive.position(centralDirectoryStartRelativeOffset);
}
}
@@ -1146,20 +1184,22 @@ public class ZipFile implements Closeable {
skipBytes(CFD_DISK_OFFSET);
shortBbuf.rewind();
IOUtils.readFully(archive, shortBbuf);
- final int diskNumberOfCFD = ZipShort.getValue(shortBuf);
+ centralDirectoryStartDiskNumber = ZipShort.getValue(shortBuf);
skipBytes(CFD_LOCATOR_RELATIVE_OFFSET);
wordBbuf.rewind();
IOUtils.readFully(archive, wordBbuf);
- final long relativeOffsetOfCFD = ZipLong.getValue(wordBuf);
+ centralDirectoryStartRelativeOffset = ZipLong.getValue(wordBuf);
((ZipSplitReadOnlySeekableByteChannel) archive)
- .position(diskNumberOfCFD, relativeOffsetOfCFD);
+ .position(centralDirectoryStartDiskNumber, centralDirectoryStartRelativeOffset);
} else {
skipBytes(CFD_LOCATOR_OFFSET);
wordBbuf.rewind();
IOUtils.readFully(archive, wordBbuf);
- archive.position(ZipLong.getValue(wordBuf));
+ centralDirectoryStartDiskNumber = 0;
+ centralDirectoryStartRelativeOffset = ZipLong.getValue(wordBuf);
+ archive.position(centralDirectoryStartRelativeOffset);
}
}
@@ -1313,6 +1353,9 @@ public class ZipFile implements Closeable {
final int extraFieldLen = ZipShort.getValue(shortBuf);
ze.setDataOffset(offset + LFH_OFFSET_FOR_FILENAME_LENGTH
+ SHORT + SHORT + fileNameLen + extraFieldLen);
+ if (ze.getDataOffset() + ze.getCompressedSize() > centralDirectoryStartOffset) {
+ throw new IOException("data for " + ze.getName() + " overlaps with central directory.");
+ }
return new int[] { fileNameLen, extraFieldLen };
}
diff --git a/src/test/java/org/apache/commons/compress/archivers/ZipTestCase.java b/src/test/java/org/apache/commons/compress/archivers/ZipTestCase.java
index 1d92fbd..ec717a1 100644
--- a/src/test/java/org/apache/commons/compress/archivers/ZipTestCase.java
+++ b/src/test/java/org/apache/commons/compress/archivers/ZipTestCase.java
@@ -416,7 +416,9 @@ public final class ZipTestCase extends AbstractTestCase {
final ZipArchiveEntry archiveEntry = new ZipArchiveEntry("fred");
archiveEntry.setUnixMode(0664);
- archiveEntry.setMethod(ZipEntry.DEFLATED);
+ archiveEntry.setMethod(ZipEntry.STORED);
+ archiveEntry.setSize(3);
+ archiveEntry.setCompressedSize(3);
zos.addRawArchiveEntry(archiveEntry, new ByteArrayInputStream("fud".getBytes()));
}