diff --git a/src/java.base/share/classes/java/util/zip/ZipEntry.java b/src/java.base/share/classes/java/util/zip/ZipEntry.java index 8c8bfeb3229..243779a7c49 100644 --- a/src/java.base/share/classes/java/util/zip/ZipEntry.java +++ b/src/java.base/share/classes/java/util/zip/ZipEntry.java @@ -78,7 +78,7 @@ public class ZipEntry implements ZipConstants, Cloneable { /** * Approximately 128 years, in milliseconds (ignoring leap years etc). * - * This establish an approximate high-bound value for DOS times in + * This establishes an approximate high-bound value for DOS times in * milliseconds since epoch, used to enable an efficient but * sufficient bounds check to avoid generating extended last modified * time entries. diff --git a/src/java.base/share/classes/java/util/zip/ZipOutputStream.java b/src/java.base/share/classes/java/util/zip/ZipOutputStream.java index f68c27b265e..f9712731a08 100644 --- a/src/java.base/share/classes/java/util/zip/ZipOutputStream.java +++ b/src/java.base/share/classes/java/util/zip/ZipOutputStream.java @@ -378,6 +378,10 @@ public class ZipOutputStream extends DeflaterOutputStream implements ZipConstant * Finishes writing the contents of the ZIP output stream without closing * the underlying stream. Use this method when applying multiple filters * in succession to the same output stream. + *

+ * A ZipException will be thrown if the combined length, after encoding, + * of the entry name, the extra field data, the entry comment and + * {@linkplain #CENHDR CEN Header size}, exceeds 65,535 bytes. * @throws ZipException if a ZIP file error has occurred * @throws IOException if an I/O exception has occurred */ @@ -398,7 +402,9 @@ public class ZipOutputStream extends DeflaterOutputStream implements ZipConstant } /** - * Closes the ZIP output stream as well as the stream being filtered. + * Closes the underlying stream and the stream being filtered after + * the contents of the ZIP output stream are fully written. + * * @throws ZipException if a ZIP file error has occurred * @throws IOException if an I/O error has occurred */ @@ -589,7 +595,8 @@ public class ZipOutputStream extends DeflaterOutputStream implements ZipConstant writeInt(csize); // compressed size writeInt(size); // uncompressed size byte[] nameBytes = zc.getBytes(e.name); - writeShort(nameBytes.length); + int nlen = nameBytes.length; + writeShort(nlen); int elen = getExtraLen(e.extra); if (hasZip64) { @@ -626,20 +633,19 @@ public class ZipOutputStream extends DeflaterOutputStream implements ZipConstant } } writeShort(elen); - byte[] commentBytes; + byte[] commentBytes = null; + int clen = 0; if (e.comment != null) { commentBytes = zc.getBytes(e.comment); - writeShort(Math.min(commentBytes.length, 0xffff)); - } else { - commentBytes = null; - writeShort(0); + clen = Math.min(commentBytes.length, 0xffff); } + writeShort(clen); // file comment length writeShort(0); // starting disk number writeShort(0); // internal file attributes (unused) // extra file attributes, used for storing posix permissions etc. writeInt(e.externalFileAttributes > 0 ? e.externalFileAttributes << 16 : 0); writeInt(offset); // relative offset of local header - writeBytes(nameBytes, 0, nameBytes.length); + writeBytes(nameBytes, 0, nlen); // take care of EXTID_ZIP64 and EXTID_EXTT if (hasZip64) { @@ -679,9 +685,17 @@ public class ZipOutputStream extends DeflaterOutputStream implements ZipConstant } } } + + // CEN header size + name length + comment length + extra length + // should not exceed 65,535 bytes per the PKWare APP.NOTE + // 4.4.10, 4.4.11, & 4.4.12. + long headerSize = (long)CENHDR + nlen + clen + elen; + if (headerSize > 0xFFFF ) { + throw new ZipException("invalid CEN header (bad header size)"); + } writeExtra(e.extra); if (commentBytes != null) { - writeBytes(commentBytes, 0, Math.min(commentBytes.length, 0xffff)); + writeBytes(commentBytes, 0, clen); } } diff --git a/test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java b/test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java index 4336377e0c8..3a5430d0573 100644 --- a/test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java +++ b/test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2024, 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 @@ -34,9 +34,7 @@ import java.io.*; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.channels.FileChannel; -import java.nio.charset.StandardCharsets; import java.time.LocalDateTime; -import java.util.Arrays; import java.util.zip.ZipEntry; import java.util.zip.ZipException; import java.util.zip.ZipFile; @@ -46,6 +44,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; public class CenSizeTooLarge { + + // Entry names produced in this test are fixed-length + public static final int NAME_LENGTH = 10; + // Maximum allowed CEN size allowed by the ZipFile implementation static final int MAX_CEN_SIZE = Integer.MAX_VALUE - ZipFile.ENDHDR - 1; @@ -59,11 +61,10 @@ public class CenSizeTooLarge { * fields respectively. The combined length of any * directory record and these three fields SHOULD NOT * generally exceed 65,535 bytes. - * - * Since ZipOutputStream does not enforce the 'combined length' clause, - * we simply use 65,535 (0xFFFF) for the purpose of this test. + *. + * Create a maximum extra field which does not exceed 65,535 bytes */ - static final int MAX_EXTRA_FIELD_SIZE = 65_535; + static final int MAX_EXTRA_FIELD_SIZE = 65_535 - ZipFile.CENHDR - NAME_LENGTH; // Data size (unsigned short) // Field size minus the leading header 'tag' and 'data size' fields (2 bytes each) @@ -72,9 +73,6 @@ public class CenSizeTooLarge { // Tag for the 'unknown' field type, specified in APPNOTE.txt 'Third party mappings' static final short UNKNOWN_ZIP_TAG = (short) 0x9902; - // Entry names produced in this test are fixed-length - public static final int NAME_LENGTH = 10; - // Use a shared LocalDateTime on all entries to save processing time static final LocalDateTime TIME_LOCAL = LocalDateTime.now(); @@ -84,16 +82,16 @@ public class CenSizeTooLarge { // The number of entries needed to exceed the MAX_CEN_SIZE static final int NUM_ENTRIES = (MAX_CEN_SIZE / CEN_HEADER_SIZE) + 1; - // Helps SparseOutputStream detect write of the last CEN entry - private static final String LAST_CEN_COMMENT = "LastCEN"; - private static final byte[] LAST_CEN_COMMENT_BYTES = LAST_CEN_COMMENT.getBytes(StandardCharsets.UTF_8); - // Expected ZipException message when the CEN does not fit in a Java byte array private static final String CEN_TOO_LARGE_MESSAGE = "invalid END header (central directory size too large)"; // Zip file to create for testing private File hugeZipFile; + private static final byte[] EXTRA_BYTES = makeLargeExtraField(); + // Helps SparseOutputStream detect write of the last CEN entry + private static final byte[] LAST_EXTRA_BYTES = makeLargeExtraField(); + /** * Create a zip file with a CEN size which does not fit within a Java byte array */ @@ -125,23 +123,18 @@ public class CenSizeTooLarge { // Set the time/date field for faster processing entry.setTimeLocal(TIME_LOCAL); - if (i == NUM_ENTRIES -1) { - // Help SparseOutputStream detect the last CEN entry write - entry.setComment(LAST_CEN_COMMENT); - } // Add the entry zip.putNextEntry(entry); - - } // Finish writing the last entry zip.closeEntry(); // Before the CEN headers are written, set the extra data on each entry - byte[] extra = makeLargeExtraField(); for (ZipEntry entry : entries) { - entry.setExtra(extra); + entry.setExtra(EXTRA_BYTES); } + // Help SparseOutputSream detect the last entry + entries[entries.length-1].setExtra(LAST_EXTRA_BYTES); } } @@ -165,7 +158,7 @@ public class CenSizeTooLarge { * Data Size (Two byte short) * Data Block (Contents depend on field type) */ - private byte[] makeLargeExtraField() { + private static byte[] makeLargeExtraField() { // Make a maximally sized extra field byte[] extra = new byte[MAX_EXTRA_FIELD_SIZE]; // Little-endian ByteBuffer for updating the header fields @@ -203,7 +196,7 @@ public class CenSizeTooLarge { // but instead simply advance the position, creating a sparse file channel.position(position); // Check for last CEN record - if (Arrays.equals(LAST_CEN_COMMENT_BYTES, 0, LAST_CEN_COMMENT_BYTES.length, b, off, len)) { + if (b == LAST_EXTRA_BYTES) { // From here on, write actual bytes sparse = false; } diff --git a/test/jdk/java/util/zip/ZipOutputStream/ZipOutputStreamMaxCenHdrTest.java b/test/jdk/java/util/zip/ZipOutputStream/ZipOutputStreamMaxCenHdrTest.java new file mode 100644 index 00000000000..286e043c225 --- /dev/null +++ b/test/jdk/java/util/zip/ZipOutputStream/ZipOutputStreamMaxCenHdrTest.java @@ -0,0 +1,169 @@ +/* + * Copyright (c) 2024, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* @test + * @bug 8336025 + * @summary Verify that ZipOutputStream throws a ZipException when the + * CEN header size + name length + comment length + extra length exceeds + * 65,535 bytes + * @run junit ZipOutputStreamMaxCenHdrTest + */ +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.io.*; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.zip.ZipEntry; +import java.util.zip.ZipException; +import java.util.zip.ZipFile; +import java.util.zip.ZipOutputStream; + +import static org.junit.jupiter.api.Assertions.*; + +public class ZipOutputStreamMaxCenHdrTest { + + // CEN header size + name length + comment length + extra length + // should not exceed 65,535 bytes per the PKWare APP.NOTE + // 4.4.10, 4.4.11, & 4.4.12. + static final int MAX_COMBINED_CEN_HEADER_SIZE = 0xFFFF; + + // Maximum possible size of name length + comment length + extra length + // for entries in order to not exceed 65,489 bytes minus 46 bytes for the CEN + // header length + static final int MAX_NAME_COMMENT_EXTRA_SIZE = + MAX_COMBINED_CEN_HEADER_SIZE - ZipFile.CENHDR; + + // Tag for the 'unknown' field type, specified in APPNOTE.txt 'Third party mappings' + static final short UNKNOWN_ZIP_TAG = (short) 0x9902; + + // ZIP file to be used by the tests + static final Path ZIP_FILE = Path.of("maxCENHdrTest.zip"); + + /** + * Clean up prior to test run + * + * @throws IOException if an error occurs + */ + @BeforeEach + public void startUp() throws IOException { + Files.deleteIfExists(ZIP_FILE); + } + + /** + * Validate a ZipException is thrown when the combined CEN Header, name + * length, comment length, and extra data length exceeds 65,535 bytes when + * the ZipOutputStream is closed. + */ + @ParameterizedTest + @ValueSource(ints = {MAX_COMBINED_CEN_HEADER_SIZE, + MAX_COMBINED_CEN_HEADER_SIZE - 1, + MAX_NAME_COMMENT_EXTRA_SIZE, + MAX_NAME_COMMENT_EXTRA_SIZE - 1}) + void setCommentTest(int length) throws IOException { + boolean expectZipException = length > MAX_NAME_COMMENT_EXTRA_SIZE; + final byte[] bytes = new byte[length]; + Arrays.fill(bytes, (byte) 'a'); + ZipEntry zipEntry = new ZipEntry(""); + // The comment length will trigger the ZipException + zipEntry.setComment(new String(bytes, StandardCharsets.UTF_8)); + boolean receivedException = writeZipEntry(zipEntry, expectZipException); + assertEquals(receivedException, expectZipException); + } + + /** + * Validate an ZipException is thrown when the combined CEN Header, name + * length, comment length, and extra data length exceeds 65,535 bytes when + * the ZipOutputStream is closed. + */ + @ParameterizedTest + @ValueSource(ints = {MAX_COMBINED_CEN_HEADER_SIZE, + MAX_COMBINED_CEN_HEADER_SIZE - 1, + MAX_NAME_COMMENT_EXTRA_SIZE, + MAX_NAME_COMMENT_EXTRA_SIZE - 1}) + void setNameTest(int length) throws IOException { + boolean expectZipException = length > MAX_NAME_COMMENT_EXTRA_SIZE; + final byte[] bytes = new byte[length]; + Arrays.fill(bytes, (byte) 'a'); + // The name length will trigger the ZipException + ZipEntry zipEntry = new ZipEntry(new String(bytes, StandardCharsets.UTF_8)); + boolean receivedException = writeZipEntry(zipEntry, expectZipException); + assertEquals(receivedException, expectZipException); + } + + /** + * Validate an ZipException is thrown when the combined CEN Header, name + * length, comment length, and extra data length exceeds 65,535 bytes when + * the ZipOutputStream is closed. + */ + @ParameterizedTest + @ValueSource(ints = {MAX_COMBINED_CEN_HEADER_SIZE, + MAX_COMBINED_CEN_HEADER_SIZE - 1, + MAX_NAME_COMMENT_EXTRA_SIZE, + MAX_NAME_COMMENT_EXTRA_SIZE - 1}) + void setExtraTest(int length) throws IOException { + boolean expectZipException = length > MAX_NAME_COMMENT_EXTRA_SIZE; + final byte[] bytes = new byte[length]; + // Little-endian ByteBuffer for updating the header fields + ByteBuffer buffer = ByteBuffer.wrap(bytes).order(ByteOrder.LITTLE_ENDIAN); + // We use the 'unknown' tag, specified in APPNOTE.TXT, 4.6.1 Third party mappings' + buffer.putShort(UNKNOWN_ZIP_TAG); + // Size of the actual (empty) data + buffer.putShort((short) (length - 2 * Short.BYTES)); + ZipEntry zipEntry = new ZipEntry(""); + // The extra data length will trigger the ZipException + zipEntry.setExtra(bytes); + boolean receivedException = writeZipEntry(zipEntry, expectZipException); + assertEquals(receivedException, expectZipException); + } + + /** + * Write a single Zip entry using ZipOutputStream + * @param zipEntry the ZipEntry to write + * @param expectZipException true if a ZipException is expected, false otherwse + * @return true if a ZipException was thrown + * @throws IOException if an error occurs + */ + private static boolean writeZipEntry(ZipEntry zipEntry, boolean expectZipException) + throws IOException { + boolean receivedException = false; + try (ZipOutputStream zos = new ZipOutputStream( + new BufferedOutputStream(Files.newOutputStream(ZIP_FILE)))) { + zos.putNextEntry(zipEntry); + if (expectZipException) { + ZipException ex = assertThrows(ZipException.class, zos::close); + assertTrue(ex.getMessage().matches(".*bad header size.*"), + "Unexpected ZipException message: " + ex.getMessage()); + receivedException = true; + } + } catch (Exception e) { + throw new RuntimeException("Received Unexpected Exception", e); + } + return receivedException; + } +}