From fff7e1ad00be07810bf948b8a6f94e83c435fa1f Mon Sep 17 00:00:00 2001 From: Lance Andersen Date: Wed, 22 Mar 2023 14:45:15 +0000 Subject: [PATCH] 8302483: Enhance ZIP performance Reviewed-by: ahgross, alanb, rhalade, coffeys --- .../share/classes/java/util/zip/ZipFile.java | 131 +++++++++++++++++- .../classes/jdk/nio/zipfs/ZipFileSystem.java | 53 ++++++- test/jdk/java/util/zip/TestExtraTime.java | 13 +- .../util/zip/ZipFile/CorruptedZipFiles.java | 4 +- 4 files changed, 194 insertions(+), 7 deletions(-) diff --git a/src/java.base/share/classes/java/util/zip/ZipFile.java b/src/java.base/share/classes/java/util/zip/ZipFile.java index e1c02759cb1..52d975005e0 100644 --- a/src/java.base/share/classes/java/util/zip/ZipFile.java +++ b/src/java.base/share/classes/java/util/zip/ZipFile.java @@ -69,6 +69,7 @@ import jdk.internal.ref.CleanerFactory; import jdk.internal.vm.annotation.Stable; import sun.nio.cs.UTF_8; import sun.nio.fs.DefaultFileSystemProvider; +import sun.security.action.GetBooleanAction; import sun.security.util.SignatureFileVerifier; import static java.util.zip.ZipConstants64.*; @@ -121,6 +122,12 @@ public class ZipFile implements ZipConstants, Closeable { */ public static final int OPEN_DELETE = 0x4; + /** + * Flag which specifies whether the validation of the Zip64 extra + * fields should be disabled + */ + private static final boolean disableZip64ExtraFieldValidation = + GetBooleanAction.privilegedGetProperty("jdk.util.zip.disableZip64ExtraFieldValidation"); /** * Opens a zip file for reading. * @@ -1199,6 +1206,16 @@ public class ZipFile implements ZipConstants, Closeable { if (entryPos + nlen > cen.length - ENDHDR) { zerror("invalid CEN header (bad header size)"); } + + int elen = CENEXT(cen, pos); + if (elen > 0 && !disableZip64ExtraFieldValidation) { + long extraStartingOffset = pos + CENHDR + nlen; + if ((int)extraStartingOffset != extraStartingOffset) { + zerror("invalid CEN header (bad extra offset)"); + } + checkExtraFields(pos, (int)extraStartingOffset, elen); + } + try { ZipCoder zcp = zipCoderForPos(pos); int hash = zcp.checkedHash(cen, entryPos, nlen); @@ -1214,7 +1231,6 @@ public class ZipFile implements ZipConstants, Closeable { // a String via zcp.toString, an Exception will be thrown int clen = CENCOM(cen, pos); if (clen > 0) { - int elen = CENEXT(cen, pos); int start = entryPos + nlen + elen; zcp.toString(cen, start, clen); } @@ -1224,6 +1240,119 @@ public class ZipFile implements ZipConstants, Closeable { return nlen; } + /** + * Validate the Zip64 Extra block fields + * @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 + * block + */ + private void checkExtraFields(int cenPos, int startingOffset, + int extraFieldLen) throws ZipException { + // Extra field Length cannot exceed 65,535 bytes per the PKWare + // APP.note 4.4.11 + if (extraFieldLen > 0xFFFF) { + zerror("invalid extra field length"); + } + // CEN Offset where this Extra field ends + int extraEndOffset = startingOffset + extraFieldLen; + if (extraEndOffset > cen.length) { + zerror("Invalid CEN header (extra data field size too long)"); + } + int currentOffset = startingOffset; + while (currentOffset < extraEndOffset) { + int tag = get16(cen, currentOffset); + currentOffset += Short.BYTES; + + int tagBlockSize = get16(cen, currentOffset); + int tagBlockEndingOffset = currentOffset + tagBlockSize; + + // The ending offset for this tag block should not go past the + // offset for the end of the extra field + if (tagBlockEndingOffset > extraEndOffset) { + zerror("Invalid CEN header (invalid zip64 extra data field size)"); + } + currentOffset += Short.BYTES; + + if (tag == ZIP64_EXTID) { + // Get the compressed size; + long csize = CENSIZ(cen, cenPos); + // Get the uncompressed size; + long size = CENLEN(cen, cenPos); + checkZip64ExtraFieldValues(currentOffset, tagBlockSize, + csize, size); + } + 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 + * @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 + * @throws ZipException if an error occurs + */ + private void checkZip64ExtraFieldValues(int off, int blockSize, long csize, + long size) + throws ZipException { + byte[] cen = this.cen; + // Validate the Zip64 Extended Information Extra Field (0x0001) + // length. + if (!isZip64ExtBlockSizeValid(blockSize)) { + 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"); + } + } + // 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"); + } + } + } + + /** + * Validate the size and contents of a Zip64 extended information field + * The order of the Zip64 fields is fixed, but the fields MUST + * only appear if the corresponding LOC or CEN field is set to 0xFFFF: + * or 0xFFFFFFFF: + * Uncompressed Size - 8 bytes + * Compressed Size - 8 bytes + * LOC Header offset - 8 bytes + * Disk Start Number - 4 bytes + * See PKWare APP.Note Section 4.5.3 for more details + * + * @param blockSize the Zip64 Extended Information Extra Field size + * @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 int getEntryHash(int index) { return entries[index]; } private int getEntryNext(int index) { return entries[index + 1]; } private int getEntryPos(int index) { return entries[index + 2]; } diff --git a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java index 705ca4fb596..03b2a3c0f57 100644 --- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java +++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java @@ -3070,6 +3070,11 @@ class ZipFileSystem extends FileSystem { if (extra == null) return; int elen = extra.length; + // Extra field Length cannot exceed 65,535 bytes per the PKWare + // APP.note 4.4.11 + if (elen > 0xFFFF) { + throw new ZipException("invalid extra field length"); + } int off = 0; int newOff = 0; boolean hasZip64LocOffset = false; @@ -3079,26 +3084,40 @@ class ZipFileSystem extends FileSystem { int tag = SH(extra, pos); int sz = SH(extra, pos + 2); pos += 4; - if (pos + sz > elen) // invalid data - break; + if (pos + sz > elen) { // invalid data + throw new ZipException("Invalid CEN header (invalid zip64 extra data field size)"); + } switch (tag) { case EXTID_ZIP64 : + // Check to see if we have a valid block size + if (!isZip64ExtBlockSizeValid(sz)) { + throw new ZipException("Invalid CEN header (invalid zip64 extra data field size)"); + } if (size == ZIP64_MINVAL) { if (pos + 8 > elen) // invalid zip64 extra break; // fields, just skip size = LL(extra, pos); + if (size < 0) { + throw new ZipException("Invalid zip64 extra block size value"); + } pos += 8; } if (csize == ZIP64_MINVAL) { if (pos + 8 > elen) break; csize = LL(extra, pos); + if (csize < 0) { + throw new ZipException("Invalid zip64 extra block compressed size value"); + } pos += 8; } if (locoff == ZIP64_MINVAL) { if (pos + 8 > elen) break; locoff = LL(extra, pos); + if (locoff < 0) { + throw new ZipException("Invalid zip64 extra block LOC offset value"); + } } break; case EXTID_NTFS: @@ -3156,6 +3175,36 @@ class ZipFileSystem extends FileSystem { extra = null; } + /** + * Validate the size and contents of a Zip64 extended information field + * The order of the Zip64 fields is fixed, but the fields MUST + * only appear if the corresponding LOC or CEN field is set to 0xFFFF: + * or 0xFFFFFFFF: + * Uncompressed Size - 8 bytes + * Compressed Size - 8 bytes + * LOC Header offset - 8 bytes + * Disk Start Number - 4 bytes + * See PKWare APP.Note Section 4.5.3 for more details + * + * @param blockSize the Zip64 Extended Information Extra Field size + * @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; + }; + } + /** * Read the LOC extra field to obtain the Info-ZIP Extended Timestamp fields * @param zipfs The Zip FS to use diff --git a/test/jdk/java/util/zip/TestExtraTime.java b/test/jdk/java/util/zip/TestExtraTime.java index a60e810df0f..0e68e764546 100644 --- a/test/jdk/java/util/zip/TestExtraTime.java +++ b/test/jdk/java/util/zip/TestExtraTime.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -29,6 +29,8 @@ */ import java.io.*; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -59,7 +61,14 @@ public class TestExtraTime { TimeZone tz = TimeZone.getTimeZone("Asia/Shanghai"); - for (byte[] extra : new byte[][] { null, new byte[] {1, 2, 3}}) { + // A structurally valid extra data example + byte[] sampleExtra = new byte[Short.BYTES*3]; + ByteBuffer.wrap(sampleExtra).order(ByteOrder.LITTLE_ENDIAN) + .putShort((short) 123) // ID: 123 + .putShort((short) Short.BYTES) // Size: 2 + .putShort((short) 42); // Data: Two bytes + + for (byte[] extra : new byte[][] { null, sampleExtra}) { // ms-dos 1980 epoch problem test0(FileTime.from(10, TimeUnit.MILLISECONDS), null, null, null, extra); diff --git a/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java b/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java index e622a266785..8885e739be1 100644 --- a/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java +++ b/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java @@ -260,7 +260,7 @@ public class CorruptedZipFiles { public void excessiveExtraFieldLength() throws IOException { short existingExtraLength = buffer.getShort(cenpos + CENEXT); buffer.putShort(cenpos+CENEXT, (short) (existingExtraLength + 1)); - assertZipException(".*bad header size.*"); + assertZipException(".*invalid zip64 extra data field size.*"); } /* @@ -271,7 +271,7 @@ public class CorruptedZipFiles { @Test public void excessiveExtraFieldLength2() throws IOException { buffer.putShort(cenpos+CENEXT, (short) 0xfdfd); - assertZipException(".*bad header size.*"); + assertZipException(".*extra data field size too long.*"); } /*