RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v2]

Jaikiran Pai jpai at openjdk.java.net
Tue Sep 29 11:47:58 UTC 2020


On Thu, 24 Sep 2020 16:36:28 GMT, Brent Christian <bchristi at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address the review comments and introduce an array size check in JarFile.getBytes() method itself
>
> src/java.base/share/classes/java/util/jar/JarFile.java line 791:
> 
>> 789:     private byte[] getBytes(ZipEntry ze) throws IOException {
>> 790:         try (InputStream is = super.getInputStream(ze)) {
>> 791:             int len = (int)ze.getSize();
> 
> I think the main issue is the casting of the 'long' value from ZipEntry.getSize() into an 'int'.  I think checking if
> the size is > the maximum array size and throwing an OOME here might be a sufficient fix on its own.

Hello Brent,

Thank you for the review and sorry about the delayed response - I was involved in a few other things so couldn't get to
this sooner.

I have taken your input and updated this patch to address this part. Specifically, I have introduced a new
`MAX_BUFFER_SIZE` within the `JarFile`. This `MAX_BUFFER_SIZE` is an actual copy of the field (and value) of
`java.io.InputStream#MAX_BUFFER_SIZE`. I have done a minor change to the javadoc of this field as compared to what is
in the javadoc of its `InputStream` counterpart. I did this so that the OOM exception message being thrown matches the
comment in this javadoc (the `InputStream` has a mismatch in its javadoc and the actual message that gets thrown).

Additionally, in this patch, the `if (len != -1 ...)` has been changed to `if (len >= 0 ...)` to prevent the code from
entering this block when `Zip64` format is involved where (from what I can gather) an uncompressed entry size value can
have 2^64 (unsigned) bytes.

-------------

PR: https://git.openjdk.java.net/jdk/pull/323



More information about the security-dev mailing list