archive/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2025-08-27 14:54:52 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

8321156: Improve the handling of invalid UTF-8 byte sequences for ZipInputStream::getNextEntry and ZipFile::getComment

Browse source 
Reviewed-by: jpai, alanb
This commit is contained in:
Lance Andersen 2024-02-26 15:31:55 +00:00
parent c042f08632
commit 20c71ceacd
3 changed files with 288 additions and 128 deletions
Download patch file Download diff file Expand all files Collapse all files

14
src/java.base/share/classes/java/util/zip/ZipFile.java
View file

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 1995, 2023, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 1995, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
@ -308,7 +308,9 @@ public class ZipFile implements ZipConstants, Closeable {
} }
/** /**
* Returns the zip file comment, or null if none. * Returns the zip file comment. If a comment does not exist or an error is
* encountered decoding the comment using the charset specified
* when opening the Zip file, then {@code null} is returned.
* *
* @return the comment string for the zip file, or null if none * @return the comment string for the zip file, or null if none
* *
@ -322,7 +324,13 @@ public class ZipFile implements ZipConstants, Closeable {
if (res.zsrc.comment == null) { if (res.zsrc.comment == null) {
return null; return null;
} }
return res.zsrc.zc.toString(res.zsrc.comment); // If there is a problem decoding the byte array which represents
// the Zip file comment, return null;
try {
return res.zsrc.zc.toString(res.zsrc.comment);
} catch (IllegalArgumentException iae) {
return null;
}
} }
} }

13
src/java.base/share/classes/java/util/zip/ZipInputStream.java
View file

@ -515,9 +515,16 @@ public class ZipInputStream extends InflaterInputStream implements ZipConstants
} }
readFully(b, 0, len); readFully(b, 0, len);
// Force to use UTF-8 if the USE_UTF8 bit is ON // Force to use UTF-8 if the USE_UTF8 bit is ON
ZipEntry e = createZipEntry(((flag & USE_UTF8) != 0) String entryName;
? ZipCoder.toStringUTF8(b, len) try {
: zc.toString(b, len)); entryName = ((flag & USE_UTF8) != 0) ?
ZipCoder.toStringUTF8(b, len)
: zc.toString(b, len);
} catch (Exception ex) {
throw (ZipException) new ZipException(
"invalid LOC header (bad entry name)").initCause(ex);
}
ZipEntry e = createZipEntry(entryName);
// now get the remaining fields for the entry // now get the remaining fields for the entry
if ((flag & 1) == 1) { if ((flag & 1) == 1) {
throw new ZipException("encrypted ZIP entry not supported"); throw new ZipException("encrypted ZIP entry not supported");

389
test/jdk/java/util/zip/ZipFile/InvalidBytesInEntryNameOrComment.java
View file

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
@ -21,163 +21,308 @@
* questions. * questions.
* *
*/ */
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.testng.annotations.BeforeTest; import java.io.FileInputStream;
import org.testng.annotations.Test;
import java.io.ByteArrayOutputStream;
import java.io.FileOutputStream; import java.io.FileOutputStream;
import java.io.IOException; import java.io.IOException;
import java.nio.ByteBuffer; import java.nio.ByteBuffer;
import java.nio.ByteOrder; import java.nio.ByteOrder;
import java.nio.channels.FileChannel; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.Arrays; import java.util.Arrays;
import java.util.zip.ZipEntry; import java.util.Formatter;
import java.util.zip.ZipException; import java.util.stream.Stream;
import java.util.zip.ZipFile; import java.util.zip.*;
import java.util.zip.ZipOutputStream;
import static org.testng.Assert.assertEquals; import static org.junit.jupiter.api.Assertions.*;
import static org.testng.Assert.expectThrows;
/** /**
* @test * @test
* @summary Validate that opening ZIP files files with invalid UTF-8 * @bug 8301873 8321156
* byte sequences in the name or comment fields fails with ZipException * @summary Validate that a ZipException is thrown when a ZIP file with
* @run testng/othervm InvalidBytesInEntryNameOrComment * invalid UTF-8 byte sequences in the name or comment fields is opened via
* ZipFile or traversed via ZipInputStream.
* Also validate that ZipFile::getComment will return null with invalid UTF-8
* byte sequences in the ZIP file comment
* @run junit InvalidBytesInEntryNameOrComment
*/ */
public class InvalidBytesInEntryNameOrComment { public class InvalidBytesInEntryNameOrComment {
// Zip file that is created and used by the test
// Offsets for navigating the CEN fields public static final Path ZIP_FILE = Path.of("BadName.zip");
private static final int EOC_OFF = 6; // Offset from EOF to find CEN offset
private static final int CEN_HDR = 45; // Size of a CEN header
private static final int NLEN = 28; // Name length
private static final int ELEN = 30; // Extra length
private static final int CLEN = 32; // Comment length
// Example invalid UTF-8 byte sequence // Example invalid UTF-8 byte sequence
private static final byte[] INVALID_UTF8_BYTE_SEQUENCE = {(byte) 0xF0, (byte) 0xA4, (byte) 0xAD}; private static final byte[] INVALID_UTF8_BYTE_SEQUENCE = {(byte) 0xF0, (byte) 0xA4, (byte) 0xAD};
// Expected error message when an invalid entry name or entry comment is
// encountered when accessing a CEN Header
private static final String CEN_BAD_ENTRY_NAME_OR_COMMENT = "invalid CEN header (bad entry name or comment)";
// Expected ZipException regex // Expected error message when an invalid entry name is encountered when
private static final String BAD_ENTRY_NAME_OR_COMMENT = "invalid CEN header (bad entry name or comment)"; // accessing a LOC Header
private static final String LOC_HEADER_BAD_ENTRY_NAME = "invalid LOC header (bad entry name)";
// Zip file comment starting offset
private static final int ZIP_FILE_COMMENT_OFFSET = 0x93;
// CEN Header offset for the entry comment to be modified
private static final int CEN_FILE_HEADER_FILE_COMMENT_STARTING_OFFSET = 0x6D;
// CEN Header offset for the entry name to be modified
private static final int CEN_FILE_HEADER_FILENAME_STARTING_OFFSET = 0x66;
// LOC Header offset for the entry name to be modified
private static final int LOC_FILE_HEADER_FILENAME_STARTING_OFFSET = 0x1e;
// CEN Entry comment
public static final String ENTRY_COMMENT = "entryComment";
// Entry name to be modified/validated
public static final String ENTRY_NAME = "entryName";
// Zip file comment to be modified/validated
public static final String ZIP_FILE_COMMENT = "ZipFileComment";
// Buffer used to massage the byte array containing the Zip File
private ByteBuffer buffer;
// Array used to copy VALID_ZIP into prior to each test run
private byte[] zipArray;
// ZIP file with invalid name field /**
private Path invalidName; * Byte array representing a valid Zip file prior modifying the CEN/LOC
* entry name, CEN entry comment or Zip file comment with an invalid
* UTF-8 byte sequence.
* See the createZipByteArray method which was used to create the original
* Zip file
* ----------------#1--------------------
* [Central Directory Header]
* 0x3a: Signature : 0x02014b50
* 0x3e: Created Zip Spec : 0x14 [2.0]
* 0x3f: Created OS : 0x0 [MS-DOS]
* 0x40: VerMadeby : 0x14 [0, 2.0]
* 0x41: VerExtract : 0x14 [2.0]
* 0x42: Flag : 0x808
* 0x44: Method : 0x8 [DEFLATED]
* 0x46: Last Mod Time : 0x58506664 [Fri Feb 16 12:51:08 EST 2024]
* 0x4a: CRC : 0xd202ef8d
* 0x4e: Compressed Size : 0x3
* 0x52: Uncompressed Size: 0x1
* 0x56: Name Length : 0x9
* 0x58: Extra Length : 0x0
* 0x5a: Comment Length : 0xc
* 0x5c: Disk Start : 0x0
* 0x5e: Attrs : 0x0
* 0x60: AttrsEx : 0x0
* 0x64: Loc Header Offset: 0x0
* 0x68: File Name : entryName
* 0x71: Comment : [entryComment]
* [Local File Header]
* 0x0: Signature : 0x04034b50
* 0x4: Version : 0x14 [2.0]
* 0x6: Flag : 0x808
* 0x8: Method : 0x8 [DEFLATED]
* 0xa: LastMTime : 0x58506664 [Fri Feb 16 12:51:08 EST 2024]
* 0xe: CRC : 0x0
* 0x12: CSize : 0x0
* 0x16: Size : 0x0
* 0x1a: Name Length : 0x9 [entryName]
* 0x1c: ExtraLength : 0x0
* 0x1e: File Name : [entryName]
* [End Central Directory Header]
* 0x7d: Signature : 0x06054b50
* 0x85: Disk Entries: 0x1
* 0x87: Total Entries: 0x1
* 0x89: CEN Size : 0x43
* 0x8d: Offset CEN : 0x3a
* 0x91: Comment Len : 0xe
* 0x93: Comment : [ZipFileComment]
*/
public static byte[] VALID_ZIP = {
(byte) 0x50, (byte) 0x4b, (byte) 0x3, (byte) 0x4, (byte) 0x14, (byte) 0x0, (byte) 0x8, (byte) 0x8,
(byte) 0x8, (byte) 0x0, (byte) 0x7d, (byte) 0x6f, (byte) 0x50, (byte) 0x58, (byte) 0x0, (byte) 0x0,
(byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0,
(byte) 0x0, (byte) 0x0, (byte) 0x9, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x65, (byte) 0x6e,
(byte) 0x74, (byte) 0x72, (byte) 0x79, (byte) 0x4e, (byte) 0x61, (byte) 0x6d, (byte) 0x65, (byte) 0x63,
(byte) 0x0, (byte) 0x0, (byte) 0x50, (byte) 0x4b, (byte) 0x7, (byte) 0x8, (byte) 0x8d, (byte) 0xef,
(byte) 0x2, (byte) 0xd2, (byte) 0x3, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x1, (byte) 0x0,
(byte) 0x0, (byte) 0x0, (byte) 0x50, (byte) 0x4b, (byte) 0x1, (byte) 0x2, (byte) 0x14, (byte) 0x0,
(byte) 0x14, (byte) 0x0, (byte) 0x8, (byte) 0x8, (byte) 0x8, (byte) 0x0, (byte) 0x7d, (byte) 0x6f,
(byte) 0x50, (byte) 0x58, (byte) 0x8d, (byte) 0xef, (byte) 0x2, (byte) 0xd2, (byte) 0x3, (byte) 0x0,
(byte) 0x0, (byte) 0x0, (byte) 0x1, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x9, (byte) 0x0,
(byte) 0x0, (byte) 0x0, (byte) 0xc, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0,
(byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0,
(byte) 0x65, (byte) 0x6e, (byte) 0x74, (byte) 0x72, (byte) 0x79, (byte) 0x4e, (byte) 0x61, (byte) 0x6d,
(byte) 0x65, (byte) 0x65, (byte) 0x6e, (byte) 0x74, (byte) 0x72, (byte) 0x79, (byte) 0x43, (byte) 0x6f,
(byte) 0x6d, (byte) 0x6d, (byte) 0x65, (byte) 0x6e, (byte) 0x74, (byte) 0x50, (byte) 0x4b, (byte) 0x5,
(byte) 0x6, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x1, (byte) 0x0, (byte) 0x1,
(byte) 0x0, (byte) 0x43, (byte) 0x0, (byte) 0x0, (byte) 0x0, (byte) 0x3a, (byte) 0x0, (byte) 0x0,
(byte) 0x0, (byte) 0xe, (byte) 0x0, (byte) 0x5a, (byte) 0x69, (byte) 0x70, (byte) 0x46, (byte) 0x69,
(byte) 0x6c, (byte) 0x65, (byte) 0x43, (byte) 0x6f, (byte) 0x6d, (byte) 0x6d, (byte) 0x65, (byte) 0x6e,
(byte) 0x74,
};
// ZIP file with invalid comment field /**
private Path invalidComment; * Delete the Zip file if it exists prior to each run and create a copy
* of the byte array representing a valid ZIP file to be used by each test run
@BeforeTest *
public void setup() throws IOException { * @throws IOException if an error occurs
// Create a ZIP file with valid name and comment fields */
byte[] templateZip = templateZIP(); @BeforeEach
public void setupTest() throws IOException {
// Create a ZIP with a CEN name field containing an invalid byte sequence Files.deleteIfExists(ZIP_FILE);
invalidName = invalidName("invalid-name.zip", templateZip); zipArray = Arrays.copyOf(VALID_ZIP, VALID_ZIP.length);
buffer = ByteBuffer.wrap(zipArray).order(ByteOrder.LITTLE_ENDIAN);
// Create a ZIP with a CEN comment field containing an invalid byte sequence
invalidComment = invalidComment("invalid-comment.zip", templateZip);
} }
/** /**
* Opening a ZipFile with an invalid UTF-8 byte sequence in * The DataProvider of CEN offsets to modify with an invalid UTF-8 byte
* the name field of a CEN file header should throw a * sequence
* ZipException with "bad entry name or comment" *
* @return Arguments used in each test run
*/
private static Stream<Arguments> CENCommentOffsets() {
return Stream.of(
// Entry's name starting offset
Arguments.of(CEN_FILE_HEADER_FILENAME_STARTING_OFFSET),
// Entry's comment starting offset
Arguments.of(CEN_FILE_HEADER_FILE_COMMENT_STARTING_OFFSET)
);
}
/**
* Validate that the original Zip file can be opened via ZipFile.
* @throws IOException if an error occurs
*/ */
@Test @Test
public void shouldRejectInvalidName() throws IOException { public void testValidEntryNameAndComment() throws IOException {
ZipException ex = expectThrows(ZipException.class, () -> { Files.write(ZIP_FILE, zipArray);
new ZipFile(invalidName.toFile()); try (ZipFile zf = new ZipFile(ZIP_FILE.toFile())) {
}); var comment = zf.getComment();
assertEquals(ex.getMessage(), BAD_ENTRY_NAME_OR_COMMENT); assertEquals(ZIP_FILE_COMMENT, comment);
}
} }
/** /**
* Opening a ZipFile with an invalid UTF-8 byte sequence in * Validate that the original Zip file can be opened and traversed via
* the comment field of a CEN file header should throw a * ZipinputStream::getNextEntry.
* ZipException with "bad entry name or comment" * @throws IOException if an error occurs
*/ */
@Test @Test
public void shouldRejectInvalidComment() throws IOException { public void traverseZipWithZipInputStreamTest() throws IOException {
ZipException ex = expectThrows(ZipException.class, () -> { Files.write(ZIP_FILE, zipArray);
new ZipFile(invalidComment.toFile()); try (ZipInputStream zis = new ZipInputStream(new FileInputStream(ZIP_FILE.toFile()))) {
}); ZipEntry ze;
assertEquals(ex.getMessage(), BAD_ENTRY_NAME_OR_COMMENT); while ((ze = zis.getNextEntry()) != null) {
} assertEquals(ENTRY_NAME, ze.getName());
}
/**
* Make a valid ZIP file used as a template for invalid files
*/
private byte[] templateZIP() throws IOException {
ByteArrayOutputStream bout = new ByteArrayOutputStream();
try (ZipOutputStream zo = new ZipOutputStream(bout)) {
ZipEntry commentEntry = new ZipEntry("file");
commentEntry.setComment("Comment");
zo.putNextEntry(commentEntry);
} }
return bout.toByteArray();
} }
/** /**
* Make a ZIP with invalid bytes in the CEN name field * Validate that a ZipException is thrown when an entry name or entry comment
* within a CEN file header contains an invalid UTF-8 byte sequence.
*
* @param offset the offset to the file name or file comment within the CEN
* file header
* @throws IOException if an error occurs
*/ */
private Path invalidName(String name, byte[] template) throws IOException { @ParameterizedTest
ByteBuffer buffer = copyTemplate(template); @MethodSource("CENCommentOffsets")
int off = cenStart(buffer); public void testInValidEntryNameOrComment(int offset)
// Name field starts here throws IOException {
int noff = off + CEN_HDR; createInvalidUTFEntryInZipFile(offset);
Throwable ex = assertThrows(ZipException.class, () ->
// Write invalid bytes {
buffer.put(noff, INVALID_UTF8_BYTE_SEQUENCE, 0, INVALID_UTF8_BYTE_SEQUENCE.length); try (ZipFile zf = new ZipFile(ZIP_FILE.toFile())) {};
return writeFile(name, buffer); }
);
assertEquals(CEN_BAD_ENTRY_NAME_OR_COMMENT, ex.getMessage());
} }
/** /**
* Make a copy of the ZIP template and wrap it in a little-endian * Validate that a null is returned from ZipFile::getComment when the
* ByteBuffer * comment contains an invalid UTF-8 byte sequence.
* @throws IOException if an error occurs
*/ */
private ByteBuffer copyTemplate(byte[] template) { @Test
return ByteBuffer.wrap(Arrays.copyOf(template, template.length)) public void testInValidZipFileComment() throws IOException {
.order(ByteOrder.LITTLE_ENDIAN); createInvalidUTFEntryInZipFile(ZIP_FILE_COMMENT_OFFSET);
} try (ZipFile zf = new ZipFile(ZIP_FILE.toFile())) {
assertNull(zf.getComment());
/**
* Make a ZIP with invalid bytes in the CEN comment field
*/
private Path invalidComment(String name, byte[] template) throws IOException {
ByteBuffer buffer = copyTemplate(template);
int off = cenStart(buffer);
// Need to skip past the length of the name and extra fields
int nlen = buffer.getShort(off + NLEN);
int elen = buffer.getShort(off + ELEN);
// Comment field starts here
int coff = off + CEN_HDR + nlen + elen;
// Write invalid bytes
buffer.put(coff, INVALID_UTF8_BYTE_SEQUENCE, 0, INVALID_UTF8_BYTE_SEQUENCE.length);
return writeFile(name, buffer);
}
/**
* Finds the offset of the start of the CEN directory
*/
private int cenStart(ByteBuffer buffer) {
return buffer.getInt(buffer.capacity() - EOC_OFF);
}
/**
* Utility to write a ByteBuffer to disk
*/
private Path writeFile(String name, ByteBuffer buffer) throws IOException {
Path zip = Path.of(name);
try (FileChannel ch = new FileOutputStream(zip.toFile()).getChannel()) {
buffer.rewind();
ch.write(buffer);
} }
return zip; }
/**
* Validate that a ZipException is thrown when an entry name
* within a LOC file header contains an invalid UTF-8 byte sequence.
* @throws IOException if an error occurs
*/
@Test
public void invalidZipInputStreamTest() throws IOException {
createInvalidUTFEntryInZipFile(LOC_FILE_HEADER_FILENAME_STARTING_OFFSET);
Throwable ex = assertThrows(ZipException.class, () ->
{
try (ZipInputStream zis =
new ZipInputStream(new FileInputStream(ZIP_FILE.toFile()))) {
zis.getNextEntry();
};
});
assertEquals(LOC_HEADER_BAD_ENTRY_NAME, ex.getMessage());
}
/**
* Utility method which modifies a Zip file starting at the specified
* offset to include an invalid UTF-8 byte sequence.
*
* @param offset starting offset within the Zip file to modify
* @throws IOException if an error occurs
*/
private void createInvalidUTFEntryInZipFile(int offset) throws IOException {
buffer.put(offset, INVALID_UTF8_BYTE_SEQUENCE, 0,
INVALID_UTF8_BYTE_SEQUENCE.length);
Files.write(ZIP_FILE, zipArray);
}
/**
* Utility method which creates the Zip file used by the tests and
* converts Zip file to byte array declaration.
*
* @throws IOException if an error occurs
*/
private void createZipByteArray() throws IOException {
ZipOutputStream zos = new ZipOutputStream(
new FileOutputStream(ZIP_FILE.toFile()));
zos.setComment(ZIP_FILE_COMMENT);
ZipEntry entry = new ZipEntry(ENTRY_NAME);
entry.setComment(ENTRY_COMMENT);
zos.putNextEntry(entry);
zos.write(new byte[1]);
zos.closeEntry();
zos.close();
// Now create the byte array entry declaration
var fooJar = Files.readAllBytes(ZIP_FILE);
var result = createByteArray(fooJar, "VALID_ZIP");
System.out.println(result);
}
/**
* Utility method which takes a byte array and converts to byte array
* declaration. For example:
* {@snippet :
* var fooJar = Files.readAllBytes(Path.of("foo.jar"));
* var result = createByteArray(fooJar,"FOOBYTES");
* System.out.println(result);
* }
*
* @param bytes A byte array used to create a byte array declaration
* @param name Name to be used in the byte array declaration
* @return The formatted byte array declaration
*/
public static String createByteArray(byte[] bytes, String name) {
StringBuilder sb = new StringBuilder(bytes.length * 5);
Formatter fmt = new Formatter(sb);
fmt.format(" public static byte[] %s = {", name);
final int linelen = 8;
for (int i = 0; i < bytes.length; i++) {
if (i % linelen == 0) {
fmt.format("%n ");
}
fmt.format(" (byte) 0x%x,", bytes[i] & 0xff);
}
fmt.format("%n };%n");
return sb.toString();
} }
} }