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 b7b15d7de0f..7281db3297b 100644 --- a/src/java.base/share/classes/java/util/zip/ZipFile.java +++ b/src/java.base/share/classes/java/util/zip/ZipFile.java @@ -1222,16 +1222,17 @@ public class ZipFile implements ZipConstants, Closeable { int nlen = CENNAM(cen, pos); int elen = CENEXT(cen, pos); int clen = CENCOM(cen, pos); - if (entryPos + nlen > cen.length - ENDHDR) { + long headerSize = (long)CENHDR + nlen + clen + elen; + // 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. Also check that current CEN header will + // not exceed the length of the CEN array + if (headerSize > 0xFFFF || pos + headerSize > cen.length - ENDHDR) { zerror("invalid CEN header (bad header size)"); } 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); + checkExtraFields(pos, entryPos + nlen, elen); } else if (elen == 0 && (CENSIZ(cen, pos) == ZIP64_MAGICVAL || CENLEN(cen, pos) == ZIP64_MAGICVAL || CENOFF(cen, pos) == ZIP64_MAGICVAL @@ -1292,7 +1293,7 @@ public class ZipFile implements ZipConstants, Closeable { int tagBlockSize = get16(cen, currentOffset); currentOffset += Short.BYTES; - int tagBlockEndingOffset = currentOffset + tagBlockSize; + long tagBlockEndingOffset = (long)currentOffset + tagBlockSize; // The ending offset for this tag block should not go past the // offset for the end of the extra field 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 1b1009f1886..6f165ec1144 100644 --- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java +++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java @@ -1593,8 +1593,13 @@ class ZipFileSystem extends FileSystem { if (method != METHOD_STORED && method != METHOD_DEFLATED) { throw new ZipException("invalid CEN header (unsupported compression method: " + method + ")"); } - if (pos + CENHDR + nlen > limit) { - throw new ZipException("invalid CEN header (bad header size)"); + long headerSize = (long)CENHDR + nlen + clen + elen; + // 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. Also check that current CEN header will + // not exceed the length of the CEN array + if (headerSize > 0xFFFF || pos + headerSize > limit) { + zerror("invalid CEN header (bad header size)"); } if (elen > 0) { checkExtraFields(cen, pos, size, csize, locoff, diskNo, @@ -1660,7 +1665,7 @@ class ZipFileSystem extends FileSystem { int tagBlockSize = SH(cen, currentOffset); currentOffset += Short.BYTES; - int tagBlockEndingOffset = currentOffset + tagBlockSize; + long tagBlockEndingOffset = (long)currentOffset + tagBlockSize; // The ending offset for this tag block should not go past the // offset for the end of the extra field diff --git a/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java b/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java index befdf4cac15..04a1157a2f8 100644 --- a/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java +++ b/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java @@ -22,7 +22,7 @@ */ /* @test - * @bug 4770745 6218846 6218848 6237956 8313765 + * @bug 4770745 6218846 6218848 6237956 8313765 8316141 * @summary test for correct detection and reporting of corrupted zip files * @author Martin Buchholz * @run junit CorruptedZipFiles @@ -262,7 +262,7 @@ public class CorruptedZipFiles { public void excessiveExtraFieldLength() throws IOException { buffer.put(cenpos+CENEXT, (byte) 0xff); buffer.put(cenpos+CENEXT+1, (byte) 0xff); - assertZipException(".*extra data field size too long.*"); + assertZipException(".*bad header size.*"); } /* @@ -273,7 +273,7 @@ public class CorruptedZipFiles { @Test public void excessiveExtraFieldLength2() throws IOException { buffer.putShort(cenpos+CENEXT, (short) 0xfdfd); - assertZipException(".*extra data field size too long.*"); + assertZipException(".*bad header size.*"); } /* diff --git a/test/jdk/jdk/nio/zipfs/CorruptedZipFilesTest.java b/test/jdk/jdk/nio/zipfs/CorruptedZipFilesTest.java new file mode 100644 index 00000000000..57287c6d8ee --- /dev/null +++ b/test/jdk/jdk/nio/zipfs/CorruptedZipFilesTest.java @@ -0,0 +1,312 @@ +/* + * Copyright (c) 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 + * 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 8316141 + * @summary test for correct detection and reporting of corrupted zip files + * @run junit CorruptedZipFilesTest + */ + + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; +import java.util.zip.ZipEntry; +import java.util.zip.ZipException; +import java.util.zip.ZipOutputStream; + +import static java.util.zip.ZipFile.*; +import static org.junit.jupiter.api.Assertions.*; + +public class CorruptedZipFilesTest { + + /* + * Byte array holding a valid template ZIP. + * + * The 'good' ZIP file has the following structure: + * + * 0000 LOCAL HEADER #1 04034B50 + * 0004 Extract Zip Spec 14 '2.0' + * 0005 Extract OS 00 'MS-DOS' + * 0006 General Purpose Flag 0808 + * [Bits 1-2] 0 'Normal Compression' + * [Bit 3] 1 'Streamed' + * [Bit 11] 1 'Language Encoding' + * 0008 Compression Method 0008 'Deflated' + * 000A Last Mod Time 567F7D07 'Fri Mar 31 15:40:14 2023' + * 000E CRC 00000000 + * 0012 Compressed Length 00000000 + * 0016 Uncompressed Length 00000000 + * 001A Filename Length 0001 + * 001C Extra Length 0000 + * 001E Filename 'x' + * 001F PAYLOAD ... + * + * 0022 STREAMING DATA HEADER 08074B50 + * 0026 CRC 8CDC1683 + * 002A Compressed Length 00000003 + * 002E Uncompressed Length 00000001 + * + * 0032 CENTRAL HEADER #1 02014B50 + * 0036 Created Zip Spec 14 '2.0' + * 0037 Created OS 00 'MS-DOS' + * 0038 Extract Zip Spec 14 '2.0' + * 0039 Extract OS 00 'MS-DOS' + * 003A General Purpose Flag 0808 + * [Bits 1-2] 0 'Normal Compression' + * [Bit 3] 1 'Streamed' + * [Bit 11] 1 'Language Encoding' + * 003C Compression Method 0008 'Deflated' + * 003E Last Mod Time 567F7D07 'Fri Mar 31 15:40:14 2023' + * 0042 CRC 8CDC1683 + * 0046 Compressed Length 00000003 + * 004A Uncompressed Length 00000001 + * 004E Filename Length 0001 + * 0050 Extra Length 0000 + * 0052 Comment Length 0000 + * 0054 Disk Start 0000 + * 0056 Int File Attributes 0000 + * [Bit 0] 0 'Binary Data' + * 0058 Ext File Attributes 00000000 + * 005C Local Header Offset 00000000 + * 0060 Filename 'x' + * + * 0061 END CENTRAL HEADER 06054B50 + * 0065 Number of this disk 0000 + * 0067 Central Dir Disk no 0000 + * 0069 Entries in this disk 0001 + * 006B Total Entries 0001 + * 006D Size of Central Dir 0000002F + * 0071 Offset to Central Dir 00000032 + * 0075 Comment Length 0000 + * + */ + private static byte[] template; + + // Copy of the template ZIP for modification by each test + private byte[] copy; + + // Litte-endian ByteBuffer for manipulating the ZIP copy + private ByteBuffer buffer; + + // Some well-known locations in the ZIP + private static int endpos, cenpos, locpos; + + // The path used when reading/writing the corrupted ZIP to disk + private Path zip = Path.of("corrupted.zip"); + + /* + * Make a sample ZIP and calculate some known offsets into this ZIP + */ + @BeforeAll + public static void setup() throws IOException { + // Make a ZIP with a single entry + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (ZipOutputStream zos = new ZipOutputStream(out)) { + ZipEntry e = new ZipEntry("x"); + zos.putNextEntry(e); + zos.write((int)'x'); + } + template = out.toByteArray(); + // ByteBuffer for reading fields from the ZIP + ByteBuffer buffer = ByteBuffer.wrap(template).order(ByteOrder.LITTLE_ENDIAN); + + // Calculate the offset of the End of central directory record + endpos = template.length - ENDHDR; + // Look up the offet of the Central directory header + cenpos = buffer.getShort(endpos + ENDOFF); + // Look up the offset of the corresponding Local file header + locpos = buffer.getShort(cenpos + CENOFF); + + // Run some sanity checks on the valid ZIP: + assertEquals(ENDSIG, buffer.getInt(endpos),"Where's ENDSIG?"); + assertEquals(CENSIG, buffer.getInt(cenpos),"Where's CENSIG?"); + assertEquals(LOCSIG, buffer.getInt(locpos),"Where's LOCSIG?"); + assertEquals(buffer.getShort(cenpos+CENNAM), + buffer.getShort(locpos+LOCNAM), + "Name field length mismatch"); + assertEquals(buffer.getShort(cenpos+CENEXT), + buffer.getShort( locpos+LOCEXT), + "Extra field length mismatch"); + } + + /* + * Make a copy safe to modify by each test + */ + @BeforeEach + public void makeCopy() { + copy = template.clone(); + buffer = ByteBuffer.wrap(copy).order(ByteOrder.LITTLE_ENDIAN); + } + + /* + * Delete the ZIP file produced after each test method + */ + @AfterEach + public void cleanup() throws IOException { + Files.deleteIfExists(zip); + } + + /* + * A ZipException is thrown when the 'End of Central Directory' + * (END) header has a CEN size exceeding past the offset of the END record + */ + @Test + public void excessiveCENSize() throws IOException { + buffer.putInt(endpos+ENDSIZ, 0xff000000); + assertZipException(".*bad central directory size.*"); + } + + /* + * A ZipException is thrown when the 'End of Central Directory' + * (END) header has a CEN offset with an invalid value. + */ + @Test + public void excessiveCENOffset() throws IOException { + buffer.putInt(endpos+ENDOFF, 0xff000000); + assertZipException(".*bad central directory offset.*"); + } + + /* + * A ZipException is thrown when a CEN header has an unexpected signature + */ + @Test + public void invalidCENSignature() throws IOException { + int existingSignature = buffer.getInt(cenpos); + buffer.putInt(cenpos, existingSignature +1); + assertZipException(".*bad signature.*"); + } + + /* + * A ZipException is thrown when a CEN header has the + * 'general purpose bit flag 0' ('encrypted') set. + */ + @Test + public void encryptedEntry() throws IOException { + copy[cenpos+CENFLG] |= 1; + assertZipException(".*encrypted entry.*"); + } + + /* + * A ZipException is thrown when a CEN header has a file name + * length which makes the CEN header overflow into the + * 'End of central directory' record. + */ + @Test + public void excessiveFileNameLength() throws IOException { + short existingNameLength = buffer.getShort(cenpos + CENNAM); + buffer.putShort(cenpos+CENNAM, (short) (existingNameLength + 1)); + assertZipException(".*bad header size.*"); + } + + /* + * A ZipException is thrown when a CEN header has a + * file name length which makes the CEN header overflow into the + * 'End of central directory' record. + */ + @Test + public void excessiveFileNameLength2() throws IOException { + buffer.putShort(cenpos + CENNAM, (short) 0xfdfd); + assertZipException(".*bad header size.*"); + } + + /* + * A ZipException is thrown if a CEN header has an + * extra field length which makes the CEN header overflow into the + * End of central directory record. + */ + @Test + public void excessiveExtraFieldLength() throws IOException { + buffer.put(cenpos+CENEXT, (byte) 0xff); + buffer.put(cenpos+CENEXT+1, (byte) 0xff); + assertZipException(".*bad header size.*"); + } + + /* + * A ZipException is thrown by Zip FS if a CEN header has an + * extra field length which makes the CEN header overflow into the + * End of central directory record. + */ + @Test + public void excessiveExtraFieldLength2() throws IOException { + buffer.putShort(cenpos+CENEXT, (short) 0xfdfd); + assertZipException(".*bad header size.*"); + } + + /* + * A ZipException is thrown when a CEN header has a comment length + * which overflows into the 'End of central directory' record + */ + @Test + public void excessiveCommentLength() throws IOException { + short existingCommentLength = buffer.getShort(cenpos + CENCOM); + buffer.putShort(cenpos+CENCOM, (short) (existingCommentLength + 1)); + assertZipException(".*bad header size.*"); + } + + /* + * A ZipException is thrown when a CEN header has a + * compression method field which is unsupported by the implementation + */ + @Test + public void unsupportedCompressionMethod() throws IOException { + copy[cenpos+CENHOW] = 2; + assertZipException(".*unsupported compression method.*"); + } + + /* + * Assert that opening a ZIP file and consuming the entry's + * InputStream using the ZipFile API fails with a ZipException + * with a message matching the given pattern. + * + * The ZIP file opened is the contents of the 'copy' byte array. + */ + void assertZipException(String msgPattern) throws IOException { + + Files.write(zip, copy); + + ZipException ex = assertThrows(ZipException.class, () -> { + try (FileSystem fs = FileSystems.newFileSystem(zip, Map.of())) { + Path p = fs.getPath("x"); + try (InputStream is = Files.newInputStream(p)) { + is.transferTo(OutputStream.nullOutputStream()); + } + } + }); + assertTrue(ex.getMessage().matches(msgPattern), + "Unexpected ZipException message: " + ex.getMessage()); + } +}