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 b7ecd1bea8f..836dda141b7 100644 --- a/src/java.base/share/classes/java/util/zip/ZipEntry.java +++ b/src/java.base/share/classes/java/util/zip/ZipEntry.java @@ -91,7 +91,10 @@ public class ZipEntry implements ZipConstants, Cloneable { */ private static final long UPPER_DOSTIME_BOUND = 128L * 365 * 24 * 60 * 60 * 1000; - + // 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. + private static final int MAX_COMBINED_CEN_HEADER_SIZE = 0xFFFF; /** * Creates a new ZIP entry with the specified name. * @@ -99,12 +102,12 @@ public class ZipEntry implements ZipConstants, Cloneable { * The entry name * * @throws NullPointerException if the entry name is null - * @throws IllegalArgumentException if the entry name is longer than - * 0xFFFF bytes + * @throws IllegalArgumentException if the combined length of the entry name + * and the {@linkplain #CENHDR CEN Header size} exceeds 65,535 bytes. */ public ZipEntry(String name) { Objects.requireNonNull(name, "name"); - if (name.length() > 0xFFFF) { + if (!isCENHeaderValid(name, null, null)) { throw new IllegalArgumentException("entry name too long"); } this.name = name; @@ -519,8 +522,10 @@ public class ZipEntry implements ZipConstants, Cloneable { * @param extra * The extra field data bytes * - * @throws IllegalArgumentException if the length of the specified - * extra field data is greater than 0xFFFF bytes + * @throws IllegalArgumentException if the combined length of the specified + * extra field data, the {@linkplain #getName() entry name}, + * the {@linkplain #getComment() entry comment}, and the + * {@linkplain #CENHDR CEN Header size} exceeds 65,535 bytes. * * @see #getExtra() */ @@ -541,7 +546,7 @@ public class ZipEntry implements ZipConstants, Cloneable { */ void setExtra0(byte[] extra, boolean doZIP64, boolean isLOC) { if (extra != null) { - if (extra.length > 0xFFFF) { + if (!isCENHeaderValid(name, extra, comment)) { throw new IllegalArgumentException("invalid extra field length"); } // extra fields are in "HeaderID(2)DataSize(2)Data... format @@ -642,16 +647,19 @@ public class ZipEntry implements ZipConstants, Cloneable { /** * Sets the optional comment string for the entry. - * - *

ZIP entry comments have maximum length of 0xffff. If the length of the - * specified comment string is greater than 0xFFFF bytes after encoding, only - * the first 0xFFFF bytes are output to the ZIP file entry. - * * @param comment the comment string - * + * @throws IllegalArgumentException if the combined length + * of the specified entry comment, the {@linkplain #getName() entry name}, + * the {@linkplain #getExtra() extra field data}, and the + * {@linkplain #CENHDR CEN Header size} exceeds 65,535 bytes. * @see #getComment() */ public void setComment(String comment) { + if (comment != null) { + if (!isCENHeaderValid(name, extra, comment)) { + throw new IllegalArgumentException("entry comment too long"); + } + } this.comment = comment; } @@ -702,4 +710,22 @@ public class ZipEntry implements ZipConstants, Cloneable { throw new InternalError(e); } } + + /** + * Initial validation that the CEN header size + name length + comment length + * + extra length do not exceed 65,535 bytes per the PKWare APP.NOTE + * 4.4.10, 4.4.11, & 4.4.12. Prior to writing out the CEN Header, + * ZipOutputStream::writeCEN will do an additional validation of the combined + * length of the fields after encoding the name and comment to a byte array. + * @param name Zip entry name + * @param extra Zip extra data + * @param comment Zip entry comment + * @return true if valid CEN Header size; false otherwise + */ + static boolean isCENHeaderValid(String name, byte[] extra, String comment) { + int clen = comment == null ? 0 : comment.length(); + int elen = extra == null ? 0 : extra.length; + long headerSize = (long)CENHDR + name.length() + clen + elen; + return headerSize <= MAX_COMBINED_CEN_HEADER_SIZE; + } } 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 f9712731a08..3fe992ab440 100644 --- a/src/java.base/share/classes/java/util/zip/ZipOutputStream.java +++ b/src/java.base/share/classes/java/util/zip/ZipOutputStream.java @@ -32,6 +32,7 @@ import java.util.Objects; import java.util.Vector; import java.util.HashSet; import static java.util.zip.ZipConstants64.*; +import static java.util.zip.ZipEntry.isCENHeaderValid; import static java.util.zip.ZipUtils.*; import sun.nio.cs.UTF_8; import sun.security.action.GetBooleanAction; @@ -262,6 +263,12 @@ public class ZipOutputStream extends DeflaterOutputStream implements ZipConstant } if (zc.isUTF8()) e.flag |= USE_UTF8; + // 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. + if (!isCENHeaderValid(e.name, e.extra, e.comment) ) { + throw new ZipException("invalid CEN header (bad header size)"); + } current = new XEntry(e, written); xentries.add(current); writeLOC(current); @@ -602,6 +609,22 @@ public class ZipOutputStream extends DeflaterOutputStream implements ZipConstant if (hasZip64) { elen += (elenZIP64 + 4);// + headid(2) + datasize(2) } + + int clen = 0; + byte[] commentBytes = null; + if (e.comment != null) { + commentBytes = zc.getBytes(e.comment); + clen = commentBytes.length; + } + + // 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)"); + } + // cen info-zip extended timestamp only outputs mtime // but set the flag for a/ctime, if present in loc int flagEXTT = 0; @@ -633,12 +656,6 @@ public class ZipOutputStream extends DeflaterOutputStream implements ZipConstant } } writeShort(elen); - byte[] commentBytes = null; - int clen = 0; - if (e.comment != null) { - commentBytes = zc.getBytes(e.comment); - clen = Math.min(commentBytes.length, 0xffff); - } writeShort(clen); // file comment length writeShort(0); // starting disk number writeShort(0); // internal file attributes (unused) @@ -686,13 +703,6 @@ 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, clen); diff --git a/test/jdk/java/util/zip/ZipEntry/MaxZipEntryFieldSizeTest.java b/test/jdk/java/util/zip/ZipEntry/MaxZipEntryFieldSizeTest.java new file mode 100644 index 00000000000..40c2234f547 --- /dev/null +++ b/test/jdk/java/util/zip/ZipEntry/MaxZipEntryFieldSizeTest.java @@ -0,0 +1,174 @@ +/* + * 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 8340553 + * @summary Verify that ZipEntry(String), ZipEntry::setComment, and + * ZipEntry::setExtra throws a IllegalArgumentException when the + * combined length of the fields, including the size of the CEN Header, + * exceeds 65,535 bytes + * @run junit MaxZipEntryFieldSizeTest + */ + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class MaxZipEntryFieldSizeTest { + + // 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 + 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 Entry name used by tests + static final String ENTRY_NAME = "EntryName"; + // Max length minus the size of the ENTRY_NAME or ENTRY_COMMENT + static final int MAX_FIELD_LEN_MINUS_ENTRY_NAME = + MAX_NAME_COMMENT_EXTRA_SIZE - 9; + + /** + * Validate an IllegalArgumentException is thrown when the + * combined length of the entry name, entry comment, entry extra data, + * and CEN Header size exceeds 65,535 bytes. + */ + @ParameterizedTest + @ValueSource(ints = {30000, 35000}) + void combinedLengthTest(int length) { + String comment = "a".repeat(length); + byte[] bytes = creatExtraData(length); + int combinedLength = ENTRY_NAME.length() + comment.length() + bytes.length; + boolean expectException = combinedLength > MAX_COMBINED_CEN_HEADER_SIZE; + System.out.printf("Combined Len= %s, exception: %s%n", combinedLength, expectException); + ZipEntry zipEntry = new ZipEntry(ENTRY_NAME); + zipEntry.setComment(comment); + // The extra data length will trigger the IllegalArgumentException + if (expectException) { + assertThrows(IllegalArgumentException.class, () -> + zipEntry.setExtra(bytes)); + } else { + zipEntry.setExtra(bytes); + } + } + + /** + * Validate an IllegalArgumentException is thrown when the comment + * length exceeds 65,489 bytes. + */ + @ParameterizedTest + @ValueSource(ints = {MAX_COMBINED_CEN_HEADER_SIZE, + MAX_NAME_COMMENT_EXTRA_SIZE, + MAX_NAME_COMMENT_EXTRA_SIZE + 1, + MAX_FIELD_LEN_MINUS_ENTRY_NAME, + MAX_FIELD_LEN_MINUS_ENTRY_NAME - 1}) + void setCommentLengthTest(int length) { + boolean expectException = length >= MAX_NAME_COMMENT_EXTRA_SIZE; + ZipEntry zipEntry = new ZipEntry(ENTRY_NAME); + String comment = "a".repeat(length); + System.out.printf("Comment Len= %s, exception: %s%n", comment.length(), expectException); + // The comment length will trigger the IllegalArgumentException + if (expectException) { + assertThrows(IllegalArgumentException.class, () -> + zipEntry.setComment(comment)); + } else { + zipEntry.setComment(comment); + } + } + + /** + * Validate an IllegalArgumentException is thrown when the name + * length exceeds 65,489 bytes. + */ + @ParameterizedTest + @ValueSource(ints = {MAX_COMBINED_CEN_HEADER_SIZE, + MAX_NAME_COMMENT_EXTRA_SIZE, + MAX_NAME_COMMENT_EXTRA_SIZE + 1, + MAX_FIELD_LEN_MINUS_ENTRY_NAME, + MAX_FIELD_LEN_MINUS_ENTRY_NAME - 1}) + void nameLengthTest(int length) { + boolean expectException = length > MAX_NAME_COMMENT_EXTRA_SIZE; + String name = "a".repeat(length); + System.out.printf("name Len= %s, exception: %s%n", name.length(), expectException); + // The name length will trigger the IllegalArgumentException + if (expectException) { + assertThrows(IllegalArgumentException.class, () -> new ZipEntry(name)); + } else { + new ZipEntry(name); + } + } + + /** + * Validate an IllegalArgumentException is thrown when the extra data + * length exceeds 65,489 bytes. + */ + @ParameterizedTest + @ValueSource(ints = {MAX_COMBINED_CEN_HEADER_SIZE, + MAX_NAME_COMMENT_EXTRA_SIZE, + MAX_NAME_COMMENT_EXTRA_SIZE + 1, + MAX_FIELD_LEN_MINUS_ENTRY_NAME, + MAX_FIELD_LEN_MINUS_ENTRY_NAME - 1}) + void setExtraLengthTest(int length) { + boolean expectException = length >= MAX_NAME_COMMENT_EXTRA_SIZE; + byte[] bytes = creatExtraData(length); + ZipEntry zipEntry = new ZipEntry(ENTRY_NAME); + System.out.printf("extra Len= %s, exception: %s%n", bytes.length, expectException); + // The extra data length will trigger the IllegalArgumentException + if (expectException) { + assertThrows(IllegalArgumentException.class, () -> zipEntry.setExtra(bytes)); + } else { + zipEntry.setExtra(bytes); + } + } + + /** + * Create the extra field data which will be passed to ZipEntry::setExtra + * @param length size of the extra data + * @return byte array containing the extra data + */ + private static byte[] creatExtraData(int length) { + 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)); + return bytes; + } +} diff --git a/test/jdk/java/util/zip/ZipOutputStream/ZipOutputStreamMaxCenHdrTest.java b/test/jdk/java/util/zip/ZipOutputStream/ZipOutputStreamMaxCenHdrTest.java deleted file mode 100644 index 286e043c225..00000000000 --- a/test/jdk/java/util/zip/ZipOutputStream/ZipOutputStreamMaxCenHdrTest.java +++ /dev/null @@ -1,169 +0,0 @@ -/* - * 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; - } -}