RFR: 8316141: Improve CEN header validation checking
Lance Andersen
lancea at openjdk.org
Tue Nov 28 20:09:08 UTC 2023
On Thu, 16 Nov 2023 20:52:08 GMT, Eirik Bjorsnos <duke at openjdk.org> wrote:
> > Regarding you comment about checking whether or not to check if the combined length of the CEN header + name length + comment length + extra length > 65K bytes, I chose to add this given the strong wording given this is a really old spec. That being said, I do not object to removing the validation if that is the overall preference.
>
> I can't claim to have a particularly strong opinion on this, the following is mostly me thinking aloud:
>
> * Given Hyrum's Law, it is conceivable that someone is currently using the extra or comment fields to attach up to 65535+65535 bytes of metadata for entires. The proposed validation will break such schemes. Does Oracle have a ZIP file corpus which could be used to identify files currently exceeding the combined length clause, just to get a sense of how rare or common this is?
> * The actual benefits of adding this validation after all these years is not quite clear to me. I don't see how this improves security, robustness, compatibility, maintainability or other ilities (apart from strictly-following-the-spec-ility :-)
> * I created a ZIP file with an entry with an extra field of the maximal expressable length of 0xFFFF. Info-ZIP's `unzip` command on MacOS did not output any warning or error when processing this file.
Yes we have a corpus search available and have exercised this patch (along with your ZipInputStream patch) without any regressions.
Given where we are in the JDK 22 cycle, going to hold off on finalizing the PR until we fork for JDK 23 and look to move this forward early on allowing for additional time to bake
-------------
PR Comment: https://git.openjdk.org/jdk/pull/16570#issuecomment-1830639162
More information about the core-libs-dev
mailing list