Skip to content

Commit 950e3a7

Browse files
author
Eirik Bjørsnøs
committedOct 9, 2024
8341625: Improve ZipFile validation of the END header
Reviewed-by: lancea
1 parent e704c05 commit 950e3a7

File tree

2 files changed

+152
-17
lines changed

2 files changed

+152
-17
lines changed
 

‎src/java.base/share/classes/java/util/zip/ZipFile.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -1605,7 +1605,7 @@ private final int readAt(byte[] buf, int off, int len, long pos)
16051605

16061606

16071607
private static class End {
1608-
int centot; // 4 bytes
1608+
long centot; // 4 bytes
16091609
long cenlen; // 4 bytes
16101610
long cenoff; // 4 bytes
16111611
long endpos; // 4 bytes
@@ -1697,7 +1697,7 @@ private End findEND() throws IOException {
16971697
// to use the end64 values
16981698
end.cenlen = cenlen64;
16991699
end.cenoff = cenoff64;
1700-
end.centot = (int)centot64; // assume total < 2g
1700+
end.centot = centot64;
17011701
end.endpos = end64pos;
17021702
} catch (IOException x) {} // no ZIP64 loc/end
17031703
return end;
@@ -1733,11 +1733,14 @@ private void initCEN(int knownTotal) throws IOException {
17331733
if (end.cenlen > MAX_CEN_SIZE) {
17341734
zerror("invalid END header (central directory size too large)");
17351735
}
1736+
if (end.centot < 0 || end.centot > end.cenlen / CENHDR) {
1737+
zerror("invalid END header (total entries count too large)");
1738+
}
17361739
cen = this.cen = new byte[(int)end.cenlen];
17371740
if (readFullyAt(cen, 0, cen.length, cenpos) != end.cenlen) {
17381741
zerror("read CEN tables failed");
17391742
}
1740-
this.total = end.centot;
1743+
this.total = Math.toIntExact(end.centot);
17411744
} else {
17421745
cen = this.cen;
17431746
this.total = knownTotal;

‎test/jdk/java/util/zip/ZipFile/EndOfCenValidation.java

+146-14
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@
2525
* @bug 8272746
2626
* @modules java.base/jdk.internal.util
2727
* @summary Verify that ZipFile rejects files with CEN sizes exceeding the implementation limit
28-
* @run testng/othervm EndOfCenValidation
28+
* @run junit/othervm EndOfCenValidation
2929
*/
3030

3131
import jdk.internal.util.ArraysSupport;
32-
import org.testng.annotations.AfterTest;
33-
import org.testng.annotations.BeforeTest;
34-
import org.testng.annotations.Test;
32+
import org.junit.jupiter.api.BeforeEach;
33+
import org.junit.jupiter.api.AfterEach;
34+
import org.junit.jupiter.api.Test;
35+
import org.junit.jupiter.params.ParameterizedTest;
36+
import org.junit.jupiter.params.provider.ValueSource;
3537

3638
import java.io.*;
3739
import java.nio.ByteBuffer;
@@ -43,12 +45,13 @@
4345
import java.nio.file.StandardOpenOption;
4446
import java.util.Arrays;
4547
import java.util.EnumSet;
48+
import java.util.HexFormat;
4649
import java.util.zip.ZipEntry;
4750
import java.util.zip.ZipException;
4851
import java.util.zip.ZipFile;
4952
import java.util.zip.ZipOutputStream;
5053

51-
import static org.testng.Assert.*;
54+
import static org.junit.jupiter.api.Assertions.*;
5255

5356
/**
5457
* This test augments {@link TestTooManyEntries}. It creates sparse ZIPs where
@@ -70,14 +73,16 @@ public class EndOfCenValidation {
7073
private static final int ENDSIZ = ZipFile.ENDSIZ; // Offset of CEN size field within ENDHDR
7174
private static final int ENDOFF = ZipFile.ENDOFF; // Offset of CEN offset field within ENDHDR
7275
// Maximum allowed CEN size allowed by ZipFile
73-
private static int MAX_CEN_SIZE = ArraysSupport.SOFT_MAX_ARRAY_LENGTH;
76+
private static final int MAX_CEN_SIZE = ArraysSupport.SOFT_MAX_ARRAY_LENGTH;
7477

7578
// Expected message when CEN size does not match file size
7679
private static final String INVALID_CEN_BAD_SIZE = "invalid END header (bad central directory size)";
7780
// Expected message when CEN offset is too large
7881
private static final String INVALID_CEN_BAD_OFFSET = "invalid END header (bad central directory offset)";
7982
// Expected message when CEN size is too large
8083
private static final String INVALID_CEN_SIZE_TOO_LARGE = "invalid END header (central directory size too large)";
84+
// Expected message when total entry count is too large
85+
private static final String INVALID_BAD_ENTRY_COUNT = "invalid END header (total entries count too large)";
8186

8287
// A valid ZIP file, used as a template
8388
private byte[] zipBytes;
@@ -86,7 +91,7 @@ public class EndOfCenValidation {
8691
* Create a valid ZIP file, used as a template
8792
* @throws IOException if an error occurs
8893
*/
89-
@BeforeTest
94+
@BeforeEach
9095
public void setup() throws IOException {
9196
zipBytes = templateZip();
9297
}
@@ -95,7 +100,7 @@ public void setup() throws IOException {
95100
* Delete big files after test, in case the file system did not support sparse files.
96101
* @throws IOException if an error occurs
97102
*/
98-
@AfterTest
103+
@AfterEach
99104
public void cleanup() throws IOException {
100105
Files.deleteIfExists(CEN_TOO_LARGE_ZIP);
101106
Files.deleteIfExists(INVALID_CEN_SIZE);
@@ -113,11 +118,11 @@ public void shouldRejectTooLargeCenSize() throws IOException {
113118

114119
Path zip = zipWithModifiedEndRecord(size, true, 0, CEN_TOO_LARGE_ZIP);
115120

116-
ZipException ex = expectThrows(ZipException.class, () -> {
121+
ZipException ex = assertThrows(ZipException.class, () -> {
117122
new ZipFile(zip.toFile());
118123
});
119124

120-
assertEquals(ex.getMessage(), INVALID_CEN_SIZE_TOO_LARGE);
125+
assertEquals(INVALID_CEN_SIZE_TOO_LARGE, ex.getMessage());
121126
}
122127

123128
/**
@@ -133,11 +138,11 @@ public void shouldRejectInvalidCenSize() throws IOException {
133138

134139
Path zip = zipWithModifiedEndRecord(size, false, 0, INVALID_CEN_SIZE);
135140

136-
ZipException ex = expectThrows(ZipException.class, () -> {
141+
ZipException ex = assertThrows(ZipException.class, () -> {
137142
new ZipFile(zip.toFile());
138143
});
139144

140-
assertEquals(ex.getMessage(), INVALID_CEN_BAD_SIZE);
145+
assertEquals(INVALID_CEN_BAD_SIZE, ex.getMessage());
141146
}
142147

143148
/**
@@ -153,11 +158,138 @@ public void shouldRejectInvalidCenOffset() throws IOException {
153158

154159
Path zip = zipWithModifiedEndRecord(size, true, 100, BAD_CEN_OFFSET_ZIP);
155160

156-
ZipException ex = expectThrows(ZipException.class, () -> {
161+
ZipException ex = assertThrows(ZipException.class, () -> {
157162
new ZipFile(zip.toFile());
158163
});
159164

160-
assertEquals(ex.getMessage(), INVALID_CEN_BAD_OFFSET);
165+
assertEquals(INVALID_CEN_BAD_OFFSET, ex.getMessage());
166+
}
167+
168+
/**
169+
* Validate that a 'Zip64 End of Central Directory' record (the END header)
170+
* where the value of the 'total entries' field is larger than what fits
171+
* in the CEN size is rejected.
172+
*
173+
* @throws IOException if an error occurs
174+
*/
175+
@ParameterizedTest
176+
@ValueSource(longs = {
177+
-1, // Negative
178+
Long.MIN_VALUE, // Very negative
179+
0x3B / 3L - 1, // Cannot fit in test ZIP's CEN
180+
MAX_CEN_SIZE / 3 + 1, // Too large to allocate int[] entries array
181+
Long.MAX_VALUE // Unreasonably large
182+
})
183+
public void shouldRejectBadTotalEntries(long totalEntries) throws IOException {
184+
/**
185+
* A small ZIP using the ZIP64 format.
186+
*
187+
* ZIP created using: "echo -n hello | zip zip64.zip -"
188+
* Hex encoded using: "cat zip64.zip | xxd -ps"
189+
*
190+
* The file has the following structure:
191+
*
192+
* 0000 LOCAL HEADER #1 04034B50
193+
* 0004 Extract Zip Spec 2D '4.5'
194+
* 0005 Extract OS 00 'MS-DOS'
195+
* 0006 General Purpose Flag 0000
196+
* 0008 Compression Method 0000 'Stored'
197+
* 000A Last Mod Time 5947AB78 'Mon Oct 7 21:27:48 2024'
198+
* 000E CRC 363A3020
199+
* 0012 Compressed Length FFFFFFFF
200+
* 0016 Uncompressed Length FFFFFFFF
201+
* 001A Filename Length 0001
202+
* 001C Extra Length 0014
203+
* 001E Filename '-'
204+
* 001F Extra ID #0001 0001 'ZIP64'
205+
* 0021 Length 0010
206+
* 0023 Uncompressed Size 0000000000000006
207+
* 002B Compressed Size 0000000000000006
208+
* 0033 PAYLOAD hello.
209+
*
210+
* 0039 CENTRAL HEADER #1 02014B50
211+
* 003D Created Zip Spec 1E '3.0'
212+
* 003E Created OS 03 'Unix'
213+
* 003F Extract Zip Spec 2D '4.5'
214+
* 0040 Extract OS 00 'MS-DOS'
215+
* 0041 General Purpose Flag 0000
216+
* 0043 Compression Method 0000 'Stored'
217+
* 0045 Last Mod Time 5947AB78 'Mon Oct 7 21:27:48 2024'
218+
* 0049 CRC 363A3020
219+
* 004D Compressed Length 00000006
220+
* 0051 Uncompressed Length FFFFFFFF
221+
* 0055 Filename Length 0001
222+
* 0057 Extra Length 000C
223+
* 0059 Comment Length 0000
224+
* 005B Disk Start 0000
225+
* 005D Int File Attributes 0001
226+
* [Bit 0] 1 Text Data
227+
* 005F Ext File Attributes 11B00000
228+
* 0063 Local Header Offset 00000000
229+
* 0067 Filename '-'
230+
* 0068 Extra ID #0001 0001 'ZIP64'
231+
* 006A Length 0008
232+
* 006C Uncompressed Size 0000000000000006
233+
*
234+
* 0074 ZIP64 END CENTRAL DIR 06064B50
235+
* RECORD
236+
* 0078 Size of record 000000000000002C
237+
* 0080 Created Zip Spec 1E '3.0'
238+
* 0081 Created OS 03 'Unix'
239+
* 0082 Extract Zip Spec 2D '4.5'
240+
* 0083 Extract OS 00 'MS-DOS'
241+
* 0084 Number of this disk 00000000
242+
* 0088 Central Dir Disk no 00000000
243+
* 008C Entries in this disk 0000000000000001
244+
* 0094 Total Entries 0000000000000001
245+
* 009C Size of Central Dir 000000000000003B
246+
* 00A4 Offset to Central dir 0000000000000039
247+
*
248+
* 00AC ZIP64 END CENTRAL DIR 07064B50
249+
* LOCATOR
250+
* 00B0 Central Dir Disk no 00000000
251+
* 00B4 Offset to Central dir 0000000000000074
252+
* 00BC Total no of Disks 00000001
253+
*
254+
* 00C0 END CENTRAL HEADER 06054B50
255+
* 00C4 Number of this disk 0000
256+
* 00C6 Central Dir Disk no 0000
257+
* 00C8 Entries in this disk 0001
258+
* 00CA Total Entries 0001
259+
* 00CC Size of Central Dir 0000003B
260+
* 00D0 Offset to Central Dir FFFFFFFF
261+
* 00D4 Comment Length 0000
262+
*/
263+
264+
byte[] zipBytes = HexFormat.of().parseHex("""
265+
504b03042d000000000078ab475920303a36ffffffffffffffff01001400
266+
2d010010000600000000000000060000000000000068656c6c6f0a504b01
267+
021e032d000000000078ab475920303a3606000000ffffffff01000c0000
268+
00000001000000b011000000002d010008000600000000000000504b0606
269+
2c000000000000001e032d00000000000000000001000000000000000100
270+
0000000000003b000000000000003900000000000000504b060700000000
271+
740000000000000001000000504b050600000000010001003b000000ffff
272+
ffff0000
273+
""".replaceAll("\n",""));
274+
275+
// Buffer to manipulate the above ZIP
276+
ByteBuffer buf = ByteBuffer.wrap(zipBytes).order(ByteOrder.LITTLE_ENDIAN);
277+
// Offset of the 'total entries' in the 'ZIP64 END CENTRAL DIR' record
278+
// Update ZIP64 entry count to a value which cannot possibly fit in the small CEN
279+
buf.putLong(0x94, totalEntries);
280+
// The corresponding END field needs the ZIP64 magic value
281+
buf.putShort(0xCA, (short) 0xFFFF);
282+
// Write the ZIP to disk
283+
Path zipFile = Path.of("bad-entry-count.zip");
284+
Files.write(zipFile, zipBytes);
285+
286+
// Verify that the END header is rejected
287+
ZipException ex = assertThrows(ZipException.class, () -> {
288+
try (var zf = new ZipFile(zipFile.toFile())) {
289+
}
290+
});
291+
292+
assertEquals(INVALID_BAD_ENTRY_COUNT, ex.getMessage());
161293
}
162294

163295
/**

0 commit comments

Comments
 (0)
Please sign in to comment.