Ramblings of a Tampa engineer

On July 17, 2023 I released Apktool 2.8.0 which was a very large release that unfortunately carried some regressions with it. Little did I know that on July 19 only a few days later I would be contending with a stressful bug (feature?) from OpenJDK that took the builds down for days.


So to start this story off - one morning I noticed wide spread build failures on the test suite between various platforms and versions of Java. When the dust settled it appeared Apktool was failing on:

  • JDK 11
  • JDK 17
  • JDK 20

With an exception that looked like this.

Caused by:
 java.util.zip.ZipException: Invalid CEN header (invalid zip64 extra data field size)
 at java.base/java.util.zip.ZipFile$Source.zerror(ZipFile.java:1728)
 at java.base/java.util.zip.ZipFile$Source.checkExtraFields(ZipFile.java:1261)
 at java.base/java.util.zip.ZipFile$Source.checkAndAddEntry(ZipFile.java:1212)
 at java.base/java.util.zip.ZipFile$Source.initCEN(ZipFile.java:1667)
 at java.base/java.util.zip.ZipFile$Source.<init>(ZipFile.java:1445)
 at java.base/java.util.zip.ZipFile$Source.get(ZipFile.java:1407)
 at java.base/java.util.zip.ZipFile$CleanableResource.<init>(ZipFile.java:716)
 at java.base/java.util.zip.ZipFile.<init>(ZipFile.java:250)
 at java.base/java.util.zip.ZipFile.<init>(ZipFile.java:179)
 at java.base/java.util.zip.ZipFile.<init>(ZipFile.java:193)
 at brut.androlib.res.Framework.installFramework(Framework.java:57)

It is pretty easy to look back at this with all the knowledge and identify the variable that must have changed, but I had just merged a few changes for Gradle and GitHub Actions so my first inclination was something at fault for one of those.

A bit of research turned up nothing for gradle-build-action 2.6.1 or Gradle 8.2.1 so I was off to GitHub Actions.

Image updates between July 17 - July 19

So I knew if builds worked on July 17, but failed on July 19 I went looking for container image updates between them. This only turned up changes for Ubuntu and Windows so it could not have been the flaw as Mac was failing as well.

At this point I figured a Java update must have occurred and stumbled unto this Consolidated JDK 17 Release Notes page. However, I was looking at the OpenJDK page and my build scripts were pointing to the Azul Zulu distribution.

I'm not sure when it happened, but there are so many versions of Java now I struggled to understand if looking at an OpenJDK change-log was the same thing as my Zulu distribution. Turns out this just cluttered my research as I stumbled upon nearly 10 different distributions and I was considering testing them all on GitHub.

So I just re-ran some really old Apktool builds that were like 3 months old and they failed as well. So ultimately I had eliminated a few variables, but ran out of time and left for work.

https://twitter.com/iBotPeaches/status/1681740291978526720

So at some point during the day I put out a tweet pointing to my open issue and hoped the Internet would help. This was the way to go as it didn't take more than 24 hours for a response on that thread that pinpointed the issue.

I was able to reproduce the issue locally. It is indeed caused by JDK 17.0.8.
The following is mentioned in the changelog:
Improved ZIP64 Extra Field Validation (JDK-8302483 (not public))
java.util.zip.ZipFile has been updated to provide additional validation of ZIP64 extra fields when opening a ZIP file. This validation may be disabled by setting the system property jdk.util.zip.disableZip64ExtraFieldValidation to true.

So thanks to evowizz that was the tip I needed to identify the issue and fixed our build scripts and got the Apktool 2.8.1 out into the wild or so I thought.

It didn't take too long though and reports in the wild were showing that this was not resolved. That didn't make sense to me because I did exactly what the description of the above change said.

System.setProperty("jdk.util.zip.disableZip64ExtraFieldValidation", "true");

I was pretty mad at this point because the change (JDK-8302483) that caused this was presumably a security measure that I knew nothing about, back-ported and released to various versions.

So I went digging and found the commit that led to this issue.

From fff7e1ad00be07810bf948b8a6f94e83c435fa1f Mon Sep 17 00:00:00 2001
From: Lance xxx <xxx@openjdk.org>
Date: Wed, 22 Mar 2023 14:45:15 +0000
Subject: [PATCH] 8302483: Enhance ZIP performance

Reviewed-by: ahgross, alanb, rhalade, coffeys
---
 .../share/classes/java/util/zip/ZipFile.java  | 131 +++++++++++++++++-
 .../classes/jdk/nio/zipfs/ZipFileSystem.java  |  53 ++++++-
 test/jdk/java/util/zip/TestExtraTime.java     |  13 +-
 .../util/zip/ZipFile/CorruptedZipFiles.java   |   4 +-
 4 files changed, 194 insertions(+), 7 deletions(-)

The commit description much like other security precautions is hiding in plain sight as this is not a performance boost. It is a further validation of the Zip specification.

I was quite sad when I took my error message and searched it.

Invalid CEN header (invalid zip64 extra data field size)

I had 5 results of that exact string in this commit so I couldn't fully isolate which path of code I was failing. I was even further confused because the applications it was failing on were just test sample applications I pulled from The Android Open Source Project. I was very confused at this point because the zip64 specification was over 20 years old and my sample applications didn't feel like applications with a 64-bit format nor a malformed one.

I started digging into the commit above more and got a bit sad. We can easily see we had 2 test file changes and 2 non-test file changes, but how did we test nearly 200 new lines of code in 17 line changes in the test suite?

In my quick research - it wasn't. I couldn't find additions to the test suite that asserted the logic of the new functions - checkExtraFields and checkZip64ExtraFieldValues

This bugged me because this author when peeking some history had good behavior of the last changes to the ZipFile class with commits like:

From f0995abe62b81cf9c96cc07caa0ac27d00c96ff1 Mon Sep 17 00:00:00 2001
From: Lance xxx <xxx@openjdk.org>
Date: Mon, 7 Mar 2022 16:10:31 +0000
Subject: [PATCH] 8280404: Unexpected exception thrown when CEN file entry
 comment length is not valid

Reviewed-by: alanb
---
 .../share/classes/java/util/zip/ZipFile.java  |  11 +-
 .../zip/ZipFile/InvalidCommentLengthTest.java | 345 ++++++++++++++++++
 2 files changed, 355 insertions(+), 1 deletion(-)
 create mode 100644 test/jdk/java/util/zip/ZipFile/InvalidCommentLengthTest.java
https://github.com/openjdk/jdk/commit/f0995abe62b81cf9c96cc07caa0ac27d00c96ff1.patch
From a020b6ba8f38fe85fb26972a51e4c1068408b1c1 Mon Sep 17 00:00:00 2001
From: Lance xxx <xxx@openjdk.org>
Date: Wed, 23 Feb 2022 16:56:50 +0000
Subject: [PATCH] 8280409: JarFile::getInputStream can fail with NPE accessing
 ze.getName()

Reviewed-by: mullan, alanb
---
 .../share/classes/java/util/jar/JarFile.java  |   33 +-
 .../share/classes/java/util/zip/ZipFile.java  |   47 +-
 .../zip/ZipFile/GetInputStreamNPETest.java    | 1055 +++++++++++++++++
 3 files changed, 1103 insertions(+), 32 deletions(-)
 create mode 100644 test/jdk/java/util/zip/ZipFile/GetInputStreamNPETest.java
https://github.com/openjdk/jdk/commit/a020b6ba8f38fe85fb26972a51e4c1068408b1c1.patch

So I was pretty confused. The last few commits to the ZipClass had an immense amount of tests in comparison to code changes. We can see above sometimes 10x the amount of test logic for a tiny amount of logic.

However, in the case I was dealing with now I was surprised to see less than 20 code change lines in the test suite for over 200 lines of core zip changes. Not to mention when the code being changed was sometimes 10+ years old it felt like something that should be tested heavily if modified.

However, I was dancing a very toxic line with those thoughts as I contribute nothing to Java and simply leverage it. So I shouldn't sway into a toxic realm where I start assigning blame - I should just help out and see if this was intended. Since I was seriously doubting that 6 of my sample applications were violating some specification.

So I googled how to report a Java bug and landed on Oracle's site. Though I knew I was using OpenJDK so that didn't seem like the place to go. I started wasting way too much time and it appeared some little nobody like me couldn't just report a bug to OpenJDK.

So then I started researching more things and apparently StackOverflow let me know that I should report a bug into Oracle's database and if they accept it - they'll assign it into both OpenJDK and Oracles database.

Attempting to submit a bug stressed me out. There was no way I could sit down and create a code sample that replicated an entire zip header of an Android application to create a failing test case.

So while I was debating that - the wonderful guys over at JADX had encountered the same issue and investigated it much further than I did. They took a more terse approach and dropped a comment to me of:

The Java developers were so "intelligent" to check this property at load-time of java.util.zip.ZipFile and copy it's value into a private static field:
private static final boolean disableZip64ExtraFieldValidation =
GetBooleanAction.privilegedGetProperty("jdk.util.zip.disableZip64ExtraFieldValidation");
As the Java class loaders use ZIPFile class to load the Jar files from class path, ZipFile class is loaded long before any user created class respectively the main class is loaded. Therefore I don't see a way how a call to System.setProperty("jdk.util.zip.disableZip64ExtraFieldValidation", "true"); will ever have any effect in a Java program. From my perspective the only way to set this property is from "outside", thus on the java command-line.

So this made sense now why my fix wasn't working. By the time Apktool's code was running - the zip class was already invoked via the class loader.

So now my only resolution was instructing all consumers of Apktool to adjust the process of how they invoke the java file or grab an updated helper script.

So now instead of java -jar apktool.jar, folks had to do java -Djdk.util.zip.disableZip64ExtraFieldValidation=true -jar apktool.jar

This must be some real urgent vulnerability for this to be not public and patched everywhere in what seems like a rushed approach. Thankfully since its a week or so later Red Hat has a bit more description of what this vulnerability was.

OpenJDK: ZIP file parsing infinite loop (8302483) (CVE-2023-22036)

Turns out we (Apktool) weren't alone in this patch causing some issues. Looking at the CVE and fix - there is no way any of the Apktool samples are malformed applications that cause infinite looping and I doubt all of the below affected projects are that as well.

So I was stressing out and just gave up on this - went to the OpenJDK commit log and made a comment.

https://github.com/openjdk/jdk/commit/fff7e1ad00be07810bf948b8a6f94e83c435fa1f#r122903240

and then a bot came in and blew out my comment with:

At this point - I gave up. I'm not trying to report a regression anymore. If Chromium was affected and corporations a lot larger than my single person - I'll let them dig into this and look for fixes in the next Java releases.

Hopefully by the time Apktool 2.8.2 comes out this will be resolved as I'm not looking forward to stacking properties onto the invocation of Java.

UPDATE (Sept 5, 2023)

It seems that we've discovered many tools of the past have been generating zip files against the specification. This increased validation has caught these mistakes, so a new JDK pull request went up to adapt this validation to be a bit less strict while still keeping the purpose of the increased validation.

You’ve successfully subscribed to Connor Tumbleson
Welcome back! You’ve successfully signed in.
Great! You’ve successfully signed up.
Success! Your email is updated.
Your link has expired
Success! Check your email for magic link to sign-in.