iyxan23 / zipalign-java

zipalign implemented as a java library with 0 deps
MIT License
53 stars 13 forks source link

Prevent shiftAmount overflow #9

Closed klecko closed 8 months ago

klecko commented 8 months ago

The variable shiftAmount is a short. Previously, it was a sum of values between 1 and alignment, which was usually 4. However, with .so aligning, it is now a sum between 1 and 4096, which is very easy to overflow, resulting in a corrupted ZIP file. I have attached a small zip with dummy .so files, that when zipaligned it results in an invalid ZIP file. I have also found this issue in real apks. Upgrading shiftAmount to an int solves the issue.

$ java -jar ./zipalign-java-1.1.1.jar ./test.zip ./test2.zip 
Aligning zip ./test.zip
Zip aligned successfully, took 6ms
$ unzip -t ./test2.zip
Archive:  ./test2.zip
error [./test2.zip]:  missing 4294901760 bytes in zipfile
  (attempting to process anyway)
error: invalid zip file with overlapped components (possible zip bomb)
iyxan23 commented 8 months ago

🤦 thanks for the catch there. I will merge this shortly after.

What do you think of adding tests to prevent things like this? But, I don't feel like including test zip files into the repo would be a great idea, they'd make cloning slower.

iyxan23 commented 8 months ago

Moved the above question to #10