Skip to content

Commit

Permalink
8339711: ZipFile.Source.initCEN needlessly reads END header
Browse files Browse the repository at this point in the history
Reviewed-by: lancea, jpai, redestad
  • Loading branch information
Eirik Bjørsnøs committed Sep 30, 2024
1 parent 180affc commit cff420d
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 12 deletions.
21 changes: 12 additions & 9 deletions src/java.base/share/classes/java/util/zip/ZipFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import jdk.internal.access.JavaUtilZipFileAccess;
import jdk.internal.access.JavaUtilJarAccess;
import jdk.internal.access.SharedSecrets;
import jdk.internal.util.ArraysSupport;
import jdk.internal.util.OperatingSystem;
import jdk.internal.perf.PerfCounter;
import jdk.internal.ref.CleanerFactory;
Expand Down Expand Up @@ -1178,14 +1179,16 @@ private static class Source {
// "META-INF/".length()
private static final int META_INF_LEN = 9;
private static final int[] EMPTY_META_VERSIONS = new int[0];
// CEN size is limited to the maximum array size in the JDK
private static final int MAX_CEN_SIZE = ArraysSupport.SOFT_MAX_ARRAY_LENGTH;

private final Key key; // the key in files
private final @Stable ZipCoder zc; // ZIP coder used to decode/encode

private int refs = 1;

private RandomAccessFile zfile; // zfile of the underlying ZIP file
private byte[] cen; // CEN & ENDHDR
private byte[] cen; // CEN
private long locpos; // position of first LOC header (usually 0)
private byte[] comment; // ZIP file comment
// list of meta entries in META-INF dir
Expand Down Expand Up @@ -1241,7 +1244,7 @@ private int checkAndAddEntry(int pos, int index)
// 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) {
if (headerSize > 0xFFFF || pos + headerSize > cen.length) {
zerror("invalid CEN header (bad header size)");
}

Expand Down Expand Up @@ -1294,7 +1297,7 @@ private void checkExtraFields(int cenPos, int startingOffset,
}
// CEN Offset where this Extra field ends
int extraEndOffset = startingOffset + extraFieldLen;
if (extraEndOffset > cen.length - ENDHDR) {
if (extraEndOffset > cen.length) {
zerror("Invalid CEN header (extra data field size too long)");
}
int currentOffset = startingOffset;
Expand Down Expand Up @@ -1732,12 +1735,12 @@ private void initCEN(int knownTotal) throws IOException {
if (locpos < 0) {
zerror("invalid END header (bad central directory offset)");
}
// read in the CEN and END
if (end.cenlen + ENDHDR >= Integer.MAX_VALUE) {
// read in the CEN
if (end.cenlen > MAX_CEN_SIZE) {
zerror("invalid END header (central directory size too large)");
}
cen = this.cen = new byte[(int)(end.cenlen + ENDHDR)];
if (readFullyAt(cen, 0, cen.length, cenpos) != end.cenlen + ENDHDR) {
cen = this.cen = new byte[(int)end.cenlen];
if (readFullyAt(cen, 0, cen.length, cenpos) != end.cenlen) {
zerror("read CEN tables failed");
}
this.total = end.centot;
Expand Down Expand Up @@ -1766,7 +1769,7 @@ private void initCEN(int knownTotal) throws IOException {
int idx = 0; // Index into the entries array
int pos = 0;
int entryPos = CENHDR;
int limit = cen.length - ENDHDR;
int limit = cen.length;
manifestNum = 0;
while (entryPos <= limit) {
if (idx >= entriesLength) {
Expand Down Expand Up @@ -1829,7 +1832,7 @@ private void initCEN(int knownTotal) throws IOException {
} else {
metaVersions = EMPTY_META_VERSIONS;
}
if (pos + ENDHDR != cen.length) {
if (pos != cen.length) {
zerror("invalid CEN header (bad header size)");
}
}
Expand Down
4 changes: 3 additions & 1 deletion test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@

/* @test
* @bug 8272746
* @modules java.base/jdk.internal.util
* @summary Verify that ZipFile rejects a ZIP with a CEN size which does not fit in a Java byte array
* @run junit CenSizeTooLarge
*/

import jdk.internal.util.ArraysSupport;
import org.junit.Before;
import org.junit.Test;

Expand All @@ -49,7 +51,7 @@ public class CenSizeTooLarge {
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;
static final int MAX_CEN_SIZE = ArraysSupport.SOFT_MAX_ARRAY_LENGTH;

/**
* From the APPNOTE.txt specification:
Expand Down
6 changes: 4 additions & 2 deletions test/jdk/java/util/zip/ZipFile/EndOfCenValidation.java
Original file line number Diff line number Diff line change
@@ -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.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -23,10 +23,12 @@

/* @test
* @bug 8272746
* @modules java.base/jdk.internal.util
* @summary Verify that ZipFile rejects files with CEN sizes exceeding the implementation limit
* @run testng/othervm EndOfCenValidation
*/

import jdk.internal.util.ArraysSupport;
import org.testng.annotations.AfterTest;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
Expand Down Expand Up @@ -68,7 +70,7 @@ public class EndOfCenValidation {
private static final int ENDSIZ = ZipFile.ENDSIZ; // Offset of CEN size field within ENDHDR
private static final int ENDOFF = ZipFile.ENDOFF; // Offset of CEN offset field within ENDHDR
// Maximum allowed CEN size allowed by ZipFile
private static int MAX_CEN_SIZE = Integer.MAX_VALUE - ENDHDR - 1;
private static int MAX_CEN_SIZE = ArraysSupport.SOFT_MAX_ARRAY_LENGTH;

// Expected message when CEN size does not match file size
private static final String INVALID_CEN_BAD_SIZE = "invalid END header (bad central directory size)";
Expand Down

1 comment on commit cff420d

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.