On Tue, 15 Aug 2023 18:43:36 GMT, Lance Andersen <lan...@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:
> 
>   Cleaned up spacing and added missing comma

> That was created manually and then repacked by the zip.

> That file is accepted by zip, by the latest JDK8u382, by the JDK20 GA, and 
> rejected by the 20.0.2. That is a regression in the latest update of JDK11+ 
> which we trying to solve here.

In my opinion we should resolve the regression for existing zip files and zip 
files which are commonly created by popular tools.

As far as I understand you can manually create "artificial" zip files which can 
be processed by the zip tool and previous versions of the JDK but not by new 
ones. As long as these kind of files aren't automatically generated by common 
tools, I don't see that as a *real* regression. I'm not even sure if we should 
fix that at all because hardly anybody is manually creating such zip files 
except maybe for attackers who intend to break the JDK.

I recommend we should instead fix the real problem as quickly as possible and 
create a new issue for potential additional improvements if you think that's 
necessary.

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

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

Reply via email to