RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v3]

[Lance Andersen]

On Mon, 14 Aug 2023 18:49:25 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>> This PR  updates the extra field validation added as part of [JDK-8302483](https://bugs.openjdk.org/browse/JDK-8302483)  to deal with issues seen with 3rd party tools/libraries where a ZipException may be encountered when opening select APK, ZIP or JAR files.  Please see refer to the links provided at the end the description for the more information ::
>> 
>>> ZipException: Invalid CEN header (invalid zip64 extra data field size)
>> 
>> 1. Extra field includes padding :
>> 
>> 
>> ----------------#1--------------------
>> [Central Directory Header]
>>   0x3374: Signature    : 0x02014b50
>>   0x3378: Created Zip Spec :    0xa [1.0]
>>   0x3379: Created OS   :    0x0 [MS-DOS]
>>   0x337a: VerMadeby    :    0xa [0, 1.0]
>>   0x337b: VerExtract   :    0xa [1.0]
>>   0x337c: Flag      :   0x800
>>   0x337e: Method     :    0x0 [STORED]
>>   0x3380: Last Mod Time  : 0x385ca437 [Thu Feb 28 20:33:46 EST 2008]
>>   0x3384: CRC       : 0x694c6952
>>   0x3388: Compressed Size :   0x624
>>   0x338c: Uncompressed Size:   0x624
>>   0x3390: Name Length   :   0x1b
>>  0x3392: Extra Length  :    0x7
>> 		[tag=0xcafe, sz=0, data= ]
>> 				->[tag=cafe, size=0]
>>   0x3394: Comment Length :    0x0
>>   0x3396: Disk Start   :    0x0
>>   0x3398: Attrs      :    0x0
>>   0x339a: AttrsEx     :    0x0
>>   0x339e: Loc Header Offset:    0x0
>>   0x33a2: File Name    : res/drawable/size_48x48.jpg
>> 
>> 
>> The extra field tag of `0xcafe` has its data size set to `0`.  and the extra length is `7`.   It is expected that you can use the  tag's data size to traverse the extra fields.
>> 
>> 
>> 2. The [BND tool](https://github.com/bndtools/bnd) added [problematic data to the extra field](https://issues.apache.org/jira/browse/FELIX-6622):
>> 
>> 
>> ----------------#359--------------------
>> [Central Directory Header]
>>    0x600b4: Signature        : 0x02014b50
>>    0x600b8: Created Zip Spec :       0x14 [2.0]
>>    0x600b9: Created OS       :        0x0 [MS-DOS]
>>    0x600ba: VerMadeby        :       0x14 [0, 2.0]
>>    0x600bb: VerExtract       :       0x14 [2.0]
>>    0x600bc: Flag             :      0x808
>>    0x600be: Method           :        0x8 [DEFLATED]
>>    0x600c0: Last Mod Time    : 0x2e418983 [Sat Feb 01 17:12:06 EST 2003]
>>    0x600c4: CRC              : 0xd8f689cb
>>    0x600c8: Compressed Size  :      0x23e
>>    0x600cc: Uncompressed Size:      0x392
>>    0x600d0: Name Length      :       0x20
>>    0x600d2: Extra Length     :        0x8
>> 		[tag=0xbfef, sz=61373, data=        
>>   0x600d4: Comment Length   ...
>
> Lance Andersen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add newline

> > > I am not understanding your point. There is a specific order for the Zip64 fields based on which fields have the Magic value. the spec does also not suggest that an empty Zip64 extra field can be written to the CEN when there is a Zip64 with data written to the LOC.
> > 
> > 
> > Yes, there is a specific order of fields that should be stored in the extended block if some of the data in the "body" is negative. But as you pointed out in this case the empty block or block bigger than necessary to store the size/csize/locoff is not prohibited by the spec. For example, take a look at the code in the [ZipEntry](https://github.com/openjdk/jdk/blob/c132176b932dd136d5c4314e08ac97d0fee7ba4d/src/java.base/share/classes/java/util/zip/ZipEntry.java#L553) where we accept any size of that block and just checked that it has required data in it.
> > If you disagree then point to the part of the spec which blocks such sizes.
> 
> This is how it is implemented by the "unzip" https://github.com/madler/zlib/blob/04f42ceca40f73e2978b50e93806c2a18c1281fc/contrib/minizip/unzip.c#L1035C68-L1035C76 , the dataSize is accepted as is.

> 4.6.2 Third-party Extra Fields MUST include a Header ID using
>    the format defined in the section of this document 
>    titled Extensible Data Fields (section 4.5).
> 
>    The Data Size field indicates the size of the following
>    data block. Programs can use this value to skip to the
>    next header block, passing over any data blocks that are
>    not of interest.

Zip -T would also report errors with a BND modified jar:

 zip -T bad.jar

>  net/n3/nanoxml/CDATAReader.class bad extra-field entry:
>        EF block length (61373 bytes) exceeds remaining EF data (4 bytes)
>  test of bad.jar FAILED
> 
> zip error: Zip file invalid, could not spawn unzip, or wrong unzip (original files unmodified)
> 

zipdetails would also fail with the above jar

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

PR Comment: https://git.openjdk.org/jdk/pull/15273#issuecomment-1678012762