Skip to content

Commit

Permalink
8314891: Additional Zip64 extra header validation
Browse files Browse the repository at this point in the history
Reviewed-by: coffeys
  • Loading branch information
Lance Andersen committed Nov 7, 2023
1 parent 0dcd730 commit 8274713
Show file tree
Hide file tree
Showing 3 changed files with 1,071 additions and 85 deletions.
109 changes: 73 additions & 36 deletions src/java.base/share/classes/java/util/zip/ZipFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -1220,17 +1220,23 @@ private int checkAndAddEntry(int pos, int index)
}
int entryPos = pos + CENHDR;
int nlen = CENNAM(cen, pos);
int elen = CENEXT(cen, pos);
int clen = CENCOM(cen, pos);
if (entryPos + nlen > cen.length - ENDHDR) {
zerror("invalid CEN header (bad header size)");
}

int elen = CENEXT(cen, pos);
if (elen > 0 && !DISABLE_ZIP64_EXTRA_VALIDATION) {
long extraStartingOffset = pos + CENHDR + nlen;
if ((int)extraStartingOffset != extraStartingOffset) {
zerror("invalid CEN header (bad extra offset)");
}
checkExtraFields(pos, (int)extraStartingOffset, elen);
} else if (elen == 0 && (CENSIZ(cen, pos) == ZIP64_MAGICVAL
|| CENLEN(cen, pos) == ZIP64_MAGICVAL
|| CENOFF(cen, pos) == ZIP64_MAGICVAL
|| CENDSK(cen, pos) == ZIP64_MAGICCOUNT)) {
zerror("Invalid CEN header (invalid zip64 extra len size)");
}

try {
Expand All @@ -1243,10 +1249,9 @@ private int checkAndAddEntry(int pos, int index)
entries[index++] = hash;
entries[index++] = next;
entries[index ] = pos;
// Validate comment if it exists
// if the bytes representing the comment cannot be converted to
// Validate comment if it exists.
// If the bytes representing the comment cannot be converted to
// a String via zcp.toString, an Exception will be thrown
int clen = CENCOM(cen, pos);
if (clen > 0) {
int start = entryPos + nlen + elen;
zcp.toString(cen, start, clen);
Expand All @@ -1259,6 +1264,7 @@ private int checkAndAddEntry(int pos, int index)

/**
* Validate the Zip64 Extra block fields
* @param cenPos The CEN offset for the current Entry
* @param startingOffset Extra Field starting offset within the CEN
* @param extraFieldLen Length of this Extra field
* @throws ZipException If an error occurs validating the Zip64 Extra
Expand All @@ -1273,7 +1279,7 @@ private void checkExtraFields(int cenPos, int startingOffset,
}
// CEN Offset where this Extra field ends
int extraEndOffset = startingOffset + extraFieldLen;
if (extraEndOffset > cen.length) {
if (extraEndOffset > cen.length - ENDHDR) {
zerror("Invalid CEN header (extra data field size too long)");
}
int currentOffset = startingOffset;
Expand Down Expand Up @@ -1302,58 +1308,89 @@ private void checkExtraFields(int cenPos, int startingOffset,
long csize = CENSIZ(cen, cenPos);
// Get the uncompressed size;
long size = CENLEN(cen, cenPos);
// Get the LOC offset
long locoff = CENOFF(cen, cenPos);
// Get the Disk Number
int diskNo = CENDSK(cen, cenPos);

checkZip64ExtraFieldValues(currentOffset, tagBlockSize,
csize, size);
csize, size, locoff, diskNo);
}
currentOffset += tagBlockSize;
}
}

/**
* Validate the Zip64 Extended Information Extra Field (0x0001) block
* size and that the uncompressed size and compressed size field
* values are not negative.
* Note: As we do not use the LOC offset or Starting disk number
* field value we will not validate them
* size; that the uncompressed size, compressed size field and LOC
* offset fields are not negative. Also make sure the field exists if
* the CEN header field is set to 0xFFFFFFFF.
* Note: As we do not use the Starting disk number field,
* we will not validate its value
* @param off the starting offset for the Zip64 field value
* @param blockSize the size of the Zip64 Extended Extra Field
* @param csize CEN header compressed size value
* @param size CEN header uncompressed size value
* @param locoff CEN header LOC offset
* @param diskNo CEN header Disk number
* @throws ZipException if an error occurs
*/
private void checkZip64ExtraFieldValues(int off, int blockSize, long csize,
long size)
long size, long locoff, int diskNo)
throws ZipException {
byte[] cen = this.cen;
// if ZIP64_EXTID blocksize == 0, which may occur with some older
// versions of Apache Ant and Commons Compress, validate csize and size
// to make sure neither field == ZIP64_MAGICVAL
if (blockSize == 0) {
if (csize == ZIP64_MAGICVAL || size == ZIP64_MAGICVAL) {
if (csize == ZIP64_MAGICVAL || size == ZIP64_MAGICVAL ||
locoff == ZIP64_MAGICVAL || diskNo == ZIP64_MAGICCOUNT) {
zerror("Invalid CEN header (invalid zip64 extra data field size)");
}
// Only validate the ZIP64_EXTID data if the block size > 0
return;
}
// Validate the Zip64 Extended Information Extra Field (0x0001)
// length.
if (!isZip64ExtBlockSizeValid(blockSize)) {
if (!isZip64ExtBlockSizeValid(blockSize, csize, size, locoff, diskNo)) {
zerror("Invalid CEN header (invalid zip64 extra data field size)");
}
// Check the uncompressed size is not negative
// Note we do not need to check blockSize is >= 8 as
// we know its length is at least 8 from the call to
// isZip64ExtBlockSizeValid()
if ((size == ZIP64_MAGICVAL)) {
if(get64(cen, off) < 0) {
zerror("Invalid zip64 extra block size value");
if (size == ZIP64_MAGICVAL) {
if ( blockSize >= Long.BYTES) {
if (get64(cen, off) < 0) {
zerror("Invalid zip64 extra block size value");
}
off += Long.BYTES;
blockSize -= Long.BYTES;
} else {
zerror("Invalid Zip64 extra block, missing size");
}
}
// Check the compressed size is not negative
if ((csize == ZIP64_MAGICVAL) && (blockSize >= 16)) {
if (get64(cen, off + 8) < 0) {
zerror("Invalid zip64 extra block compressed size value");
if (csize == ZIP64_MAGICVAL) {
if (blockSize >= Long.BYTES) {
if (get64(cen, off) < 0) {
zerror("Invalid zip64 extra block compressed size value");
}
off += Long.BYTES;
blockSize -= Long.BYTES;
} else {
zerror("Invalid Zip64 extra block, missing compressed size");
}
}
// Check the LOC offset is not negative
if (locoff == ZIP64_MAGICVAL) {
if (blockSize >= Long.BYTES) {
if (get64(cen, off) < 0) {
zerror("Invalid zip64 extra block LOC OFFSET value");
}
// Note: We do not need to adjust the following fields as
// this is the last field we are leveraging
// off += Long.BYTES;
// blockSize -= Long.BYTES;
} else {
zerror("Invalid Zip64 extra block, missing LOC offset value");
}
}
}
Expand All @@ -1370,22 +1407,22 @@ private void checkZip64ExtraFieldValues(int off, int blockSize, long csize,
* See PKWare APP.Note Section 4.5.3 for more details
*
* @param blockSize the Zip64 Extended Information Extra Field size
* @param csize CEN header compressed size value
* @param size CEN header uncompressed size value
* @param locoff CEN header LOC offset
* @param diskNo CEN header Disk number
* @return true if the extra block size is valid; false otherwise
*/
private static boolean isZip64ExtBlockSizeValid(int blockSize) {
/*
* As the fields must appear in order, the block size indicates which
* fields to expect:
* 8 - uncompressed size
* 16 - uncompressed size, compressed size
* 24 - uncompressed size, compressed sise, LOC Header offset
* 28 - uncompressed size, compressed sise, LOC Header offset,
* and Disk start number
*/
return switch(blockSize) {
case 8, 16, 24, 28 -> true;
default -> false;
};
private static boolean isZip64ExtBlockSizeValid(int blockSize, long csize,
long size, long locoff,
int diskNo) {
int expectedBlockSize =
(csize == ZIP64_MAGICVAL ? Long.BYTES : 0) +
(size == ZIP64_MAGICVAL ? Long.BYTES : 0) +
(locoff == ZIP64_MAGICVAL ? Long.BYTES : 0) +
(diskNo == ZIP64_MAGICCOUNT ? Integer.BYTES : 0);
return expectedBlockSize == blockSize;

}
private int getEntryHash(int index) { return entries[index]; }
private int getEntryNext(int index) { return entries[index + 1]; }
Expand Down
Loading

1 comment on commit 8274713

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.