ZipAlign creates corrupted APK for already aligned APK

[MuntashirAkon]

Title says it all. In case you're wondering why it's necessary, we don't know if an APK is aligned or not in first place. So, we need to do that for both aligned and unaligned APKs.

[MuntashirAkon]

I am also reiterating the need for #6. Adding tests would greatly reduce issues like this. I feel that most issues I opened here could've been resolved if there were adequate tests.

[Axelen123]

I haven't encountered this myself, but my guess is that it occurs because this line assumes all compressed entries have a data descriptor, which may not always be the case.

[MuntashirAkon]

Well, I need to look into this. But the zipalign library has an option to verify if an APK is zip aligned. Maybe that could be implemented apart from this.

Here's the source code: https://github.com/MuntashirAkon/zipalign-android/blob/master/zipalign/src/main/cpp/zipalign/ZipAlign.cpp#L124-L168

Seems quite easy to implement. I can implement that if you want.

[MuntashirAkon]

I haven't encountered this myself, but my guess is that it occurs because this line assumes all compressed entries have a data descriptor, which may not always be the case.

Well, I've tried about a dozen APK files, each of them has this issue. So, I believe that most of them do not have a data descriptor. But is there a way to actually detect this using the ZipFile API? I don't see any option to do that. And how it is that it only affects signed APK files, and not unsigned ones?

[REAndroid]

ZipAlign is borrowed class and I am not full understood it. But my guess is that at line 149 byte[] extra = entry.getExtra(); is always null.

Now I am focusing on new archive management lib archive2 which also solves such issue, until then hopefully someone will find a solution. @Axelen123 @Kirlif

[Axelen123]

Well, I've tried about a dozen APK files, each of them has this issue. So, I believe that most of them do not have a data descriptor. But is there a way to actually detect this using the ZipFile API? I don't see any option to do that.

I dont think there is, since the Java Zip library does not expose flags. REAndroids guess is also plausible.

Pull request #21 uses a different zip implementation, so its possible its not affected by the issue. I havent tested the static methods on ZipAlign much, but if you do test it only to find that its broken, please let me know

[REAndroid]

Oh @Axelen123 is right, the problem was "flag" is calculated based on assumption that all non-STORED entries have data descriptor and counts 16 bytes. But non-STORED entries will not always have data descriptor.

Until archive2 ready , here is my ugly commit

[MuntashirAkon]

Until archive2 ready , here is my ugly commit

It didn't work. It still fails with the following error:

java.util.zip.DataFormatException: invalid stored block lengths
        at java.util.zip.Inflater.inflateBytes(Native Method)
        at java.util.zip.Inflater.inflate(Inflater.java:278)
        at java.util.zip.Inflater.inflate(Inflater.java:299)

[REAndroid]

• Are you testing on android os? Maybe getFlag(ZipEntry) is returning null.

Anyways if you are interested, archive2 is ready now for early experiment

[MuntashirAkon]

• Are you testing on android os? Maybe getFlag(ZipEntry) is returning null.

No, hidden API enforcement policies are bypassed. It's something to do with calculating the offsets, I think.

[MuntashirAkon]

if you are interested, archive2 is ready now for early experiment

Getting NPE for some APKs:

java.lang.NullPointerException: Attempt to invoke virtual method 'long com.reandroid.archive2.block.CentralEntryHeader.getCrc()' on a null object reference at com.reandroid.archive2.block.LocalFileHeader.mergeZeroValues(LocalFileHeader.java:32) at com.reandroid.archive2.model.LocalFileDirectory.visitLocalFile(LocalFileDirectory.java:63) at com.reandroid.archive2.model.LocalFileDirectory.visitLocalFile(LocalFileDirectory.java:48) at com.reandroid.archive2.model.LocalFileDirectory.visit(LocalFileDirectory.java:42) at com.reandroid.archive2.Archive.(Archive.java:47) at com.reandroid.archive2.Archive.(Archive.java:62)

Let me know if you need a sample.

[MuntashirAkon]

Another similar NPE:

java.lang.NullPointerException: Attempt to invoke virtual method 'com.reandroid.archive2.block.CommonHeader$GeneralPurposeFlag com.reandroid.archive2.block.CentralEntryHeader.getGeneralPurposeFlag()' on a null object reference
 at com.reandroid.archive2.block.LocalFileHeader.mergeZeroValues(LocalFileHeader.java:41)
 at com.reandroid.archive2.model.LocalFileDirectory.visitLocalFile(LocalFileDirectory.java:63)
 at com.reandroid.archive2.model.LocalFileDirectory.visitLocalFile(LocalFileDirectory.java:48)
 at com.reandroid.archive2.model.LocalFileDirectory.visit(LocalFileDirectory.java:42)
 at com.reandroid.archive2.Archive.<init>(Archive.java:47)
 at com.reandroid.archive2.Archive.<init>(Archive.java:62)

[MuntashirAkon]

The new API works great except the aforementioned cases. BTW, here's the class I use for verification (it was converted from the ZipAlign library for Android):

ZipAlignVerifier.java

```java public class ZipAlignVerifier { public static final String TAG = ZipAlignVerifier.class.getSimpleName(); public static final int kPageAlignment = 4096; public static boolean verify(File file, int alignment, boolean pageAlignSharedLibs) { Archive zipFile; boolean foundBad = false; Log.d(TAG, String.format(Locale.ROOT, "Verifying alignment of %s (%d)...\n", file, alignment)); try { zipFile = new Archive(file); } catch (IOException e) { Log.e(TAG, String.format(Locale.ROOT, "Unable to open '%s' for verification\n", file), e); return false; } List entries = zipFile.getEntryList(); long lastFileOffset; for (ArchiveEntry pEntry : entries) { String name = pEntry.getName(); lastFileOffset = pEntry.getFileOffset(); if (pEntry.getMethod() == ZipEntry.DEFLATED) { Log.d(TAG, String.format(Locale.ROOT, "%8d %s (OK - compressed)\n", lastFileOffset, name)); } else if (pEntry.isDirectory()) { // Directory entries do not need to be aligned. Log.d(TAG, String.format(Locale.ROOT, "%8d %s (OK - directory)\n", lastFileOffset, name)); } else { int alignTo = getAlignment(pageAlignSharedLibs, alignment, pEntry); if ((lastFileOffset % alignTo) != 0) { Log.w(TAG, String.format(Locale.ROOT, "%8d %s (BAD - %d)\n", lastFileOffset, name, (lastFileOffset % alignTo))); foundBad = true; break; } else { Log.d(TAG, String.format(Locale.ROOT, "%8d %s (OK)\n", lastFileOffset, name)); } } } Log.d(TAG, String.format(Locale.ROOT, "Verification %s\n", foundBad ? "FAILED" : "successful")); return !foundBad; } private static int getAlignment(boolean pageAlignSharedLibs, int defaultAlignment, ZipEntry pEntry) { if (!pageAlignSharedLibs) { return defaultAlignment; } if (pEntry.getName().endsWith(".so")) { return kPageAlignment; } return defaultAlignment; } } ```

[REAndroid]

Thanks I was hopping someone to test it on android os, how is the performance relative to old API ?

[REAndroid]

Let me know if you need a sample.

Yes please

[MuntashirAkon]

Yes please

app-release.apk.zip

(Just remove the zip extension)

[REAndroid]

app-release.apk.zip

Is this compressed by archive2 ?

[MuntashirAkon]

Is this compressed by archive2 ?

No, this is just an example APK file which archive2 fails to open.

[REAndroid]

Nice check this commit and this

[MuntashirAkon]

All working fine with ZipAlignVerifier. So far, I've only tested reading the archive files. I don't know about writing though.

[MuntashirAkon]

Here's the method I used for aligning an APK file:

    public static void align(File input, File output) throws IOException {
        File dir = output.getParentFile();
        if (dir != null && !dir.exists()) {
            dir.mkdirs();
        }
        Archive archive = new Archive(input);
        try (ApkWriter apkWriter = new ApkWriter(output, archive.mapEntrySource().values())) {
            apkWriter.write();
        }
        if (!ZipAlignVerifier.verify(output, 4, true)) {
            throw new IOException("Could not verify aligned APK file.");
        }
    }

Before aligning the file, the verifier outputs the following error:

      39 .DS_Store (OK - compressed)
     480 AndroidManifest.xml (OK - compressed)
    2976 classes.dex (OK - compressed)
 1485993 lib/.DS_Store (OK - compressed)
 1486449 lib/arm64-v8a/libcmph.so (BAD - 3697)
Verification FAILED

After aligning the file, I still get a failure:

      39 .DS_Store (OK - compressed)
     480 AndroidManifest.xml (OK - compressed)
    2976 classes.dex (OK - compressed)
 1485993 lib/.DS_Store (OK - compressed)
 1486902 lib/arm64-v8a/libcmph.so (BAD - 54)
Verification FAILED

Conclusion Alignment in archive2 is not working.

[REAndroid]

Thanks for your excellent testing report! As on this commit @Axelen123 suggested that all native libraries must have alignment of 4096

You can control each file alignment as:

    public static void align(File input, File output) throws IOException {
        File dir = output.getParentFile();
        if (dir != null && !dir.exists()) {
            dir.mkdirs();
        }        
        Archive archive = new Archive(input);
        ApkWriter apkWriter = new ApkWriter(output, archive.mapEntrySource().values());

        ZipAligner zipAligner = new ZipAligner();
        zipAligner.setDefaultAlignment(4);
        zipAligner.clearFileAlignment();

        // see ZipAligner.apkAligner();

        apkWriter.write();
        if (!ZipAlignVerifier.verify(output, 4, true)) {
            throw new IOException("Could not verify aligned APK file.");
        }
    }

NB: Updated with this commit

[Axelen123]

The aligner for archive2 should work properly in #31.

[MuntashirAkon]

Getting this error:

java.lang.ArrayIndexOutOfBoundsException: src.length=102400 srcPos=0 dst.length=16 dstPos=0 length=-1
    at java.lang.System.arraycopy(System.java:524)
    at com.reandroid.archive2.io.FileChannelInputStream.readBuffer(FileChannelInputStream.java:84)
    at com.reandroid.archive2.io.FileChannelInputStream.read(FileChannelInputStream.java:62)
    at com.reandroid.archive2.block.ZipHeader.readBasic(ZipHeader.java:45)
    at com.reandroid.archive2.block.ZipHeader.readBytes(ZipHeader.java:33)
    at com.reandroid.archive2.model.LocalFileDirectory.visitLocalFile(LocalFileDirectory.java:68)
    at com.reandroid.archive2.model.LocalFileDirectory.visit(LocalFileDirectory.java:41)
    at com.reandroid.archive2.Archive.<init>(Archive.java:47)
    at com.reandroid.archive2.Archive.<init>(Archive.java:62)

[REAndroid]

Did you checked this commit

[MuntashirAkon]

Did you checked this commit

Thanks. All known issues seem to have been fixed.

[MuntashirAkon]

For future reference, the ZipAlign helper class I've used in my project is located here. The API is identical to the AOSP zipalign.