RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName()
Lance Andersen
lancea at openjdk.java.net
Fri Feb 4 15:22:06 UTC 2022
On Fri, 4 Feb 2022 15:06:59 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> Hi all,
>>
>> Please review the attached patch to address
>>
>> - That JarFile::getInputStream did not check for a null ZipEntry passed as a parameter
>> - Have Zip/JarFile::getInputStream throw a ZipException in the event that an unexpected exception occurs
>>
>> Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar test runs
>>
>> Best
>> Lance
>
> src/java.base/share/classes/java/util/jar/JarFile.java line 840:
>
>> 838: throws IOException
>> 839: {
>> 840: Objects.requireNonNull(ze, "ze");
>
> Is the NPE specified?
Yes it is specified in the JarFile main paragraph
`Unless otherwise noted, passing a null argument to a constructor or method in this class will cause a NullPointerException to be thrown.`
ZipFile::getInputStream validates the argument as soon as it is passed to getInputStream.
> src/java.base/share/classes/java/util/jar/JarFile.java line 866:
>
>> 864: } catch (Exception e2) {
>> 865: // Any other Exception should be a ZipException
>> 866: throw (ZipException) new ZipException("Zip file format error").initCause(e2);
>
> If there is ZIP format error then I would expect ZipException or the more general IOException is already thrown. So I think this is catching other cases, maybe broken manifests or signed JAR files, in which case a JarException may be better.
JarFile::getInputStream. mentions ZipException but not JarException which is why I chose this. If we change this to JarException, I would need to update the javadoc and create a CSR.
Please let me know your preference
-------------
PR: https://git.openjdk.java.net/jdk/pull/7348
More information about the security-dev
mailing list