RFR: 8340553: ZipEntry field validation does not take into account the size of a CEN header [v9]

Lance Andersen lancea at openjdk.org
Sat Oct 19 15:55:01 UTC 2024


On Sat, 19 Oct 2024 15:38:26 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Lance Andersen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add Combined test
>
> src/java.base/share/classes/java/util/zip/ZipEntry.java line 723:
> 
>> 721:      * @return true if valid CEN Header size; false otherwise
>> 722:      */
>> 723:      static boolean isCENHeaderValid(String name, byte[] extra, String comment) {
> 
> I think it should be spelled out better that `isCENHeaderValid` can give false positives since both `name` and `comment` may turn into more than `String::length` bytes after conversion. So while it's a reasonable sanity check to fail-fast you still need an exact check after conversion like in `ZipOutputStream::writeCEN`. Or we could consider converting `name` and `comment` to bytes eagerly and get an exact check up front?

Sure, I can add an additional comment here if you like for future maintainers.  I don't want to consider converting name/comments earlier than needed as part of this PR as that start to creep out of the scope of the original issue being addressed, especially given this is a corner case(a corpus search of 90K+ jars only found 520 Zip entries with a header size between 500 - 1000 bytes and everything else  < 500 bytes).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21544#discussion_r1807391884


More information about the core-libs-dev mailing list