On Mon, 7 Oct 2024 09:13:57 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
> Please review this PR which adds validation of the 'total entries' value when
> fetched from the 'ZIP64 End of Central Directory' header.
>
> We should reject this value under the following conditions:
>
> 1. It is too large to fit within the specified CEN size (considering each CEN
> header encodes as at least 46 bytes each)
> 2. It is too large to create the `int[] entries` array safely (max value is
> `ArraysSupport.SOFT_MAX_ARRAY_LENGTH / 3`)
>
> I claim that condition 2 here is already implicitly validated by the current
> maximum CEN size validation. (A CEN encoding such a large number of entries
> would exceed the maximum CEN size a lot and would already be rejected)
>
> This change aims to protect the integrity of the implementation against
> specially crafted ZIP files. No sane ZIP tool will produce such files.
>
> Testing:
>
> This PR adds a test `EndOfCenValidation.shouldRejectBadTotalEntries` which
> exercises the first condition above.
>
> ZIP tests run locally. GHA results pending.
Marked as reviewed by lancea (Reviewer).
test/jdk/java/util/zip/ZipFile/EndOfCenValidation.java line 264:
> 262: */
> 263:
> 264: byte[] zipBytes = HexFormat.of().parseHex("""
It might be beneficial to future readers to provide the steps used to create
the hex string in the format you have provided.
-------------
PR Review: https://git.openjdk.org/jdk/pull/21384#pullrequestreview-2352700634
PR Review Comment: https://git.openjdk.org/jdk/pull/21384#discussion_r1790720612
