[BUG] Expected: 0x00000008, got: 0x00000005 - WhatsApp

shifoc commented 1 year ago

Information

Apktool Version (apktool -version) -2.7.0
Operating System (Mac, Linux, Windows) -Windows/Linux
APK From? (Playstore, ROM, Other) -Playstore

Stacktrace/Logcat

apktool w.apk
I: Using Apktool 2.7.0 on w.apk
I: Loading resource table...
Exception in thread "main" brut.androlib.AndrolibException: Could not decode arsc file
        at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:56)
        at brut.androlib.res.AndrolibResources.getResPackagesFromApk(AndrolibResources.java:780)
        at brut.androlib.res.AndrolibResources.loadMainPkg(AndrolibResources.java:64)
        at brut.androlib.res.AndrolibResources.getResTable(AndrolibResources.java:56)
        at brut.androlib.Androlib.getResTable(Androlib.java:74)
        at brut.androlib.ApkDecoder.getResTable(ApkDecoder.java:251)
        at brut.androlib.ApkDecoder.decode(ApkDecoder.java:109)
        at brut.apktool.Main.cmdDecode(Main.java:175)
        at brut.apktool.Main.main(Main.java:79)
Caused by: java.io.IOException: Expected: 0x00000008, got: 0x00000005
        at brut.util.ExtDataInput.skipCheckShort(ExtDataInput.java:53)
        at brut.androlib.res.decoder.ARSCDecoder.readValue(ARSCDecoder.java:399)
        at brut.androlib.res.decoder.ARSCDecoder.readEntryData(ARSCDecoder.java:324)
        at brut.androlib.res.decoder.ARSCDecoder.readTableType(ARSCDecoder.java:309)
        at brut.androlib.res.decoder.ARSCDecoder.readTableTypeSpec(ARSCDecoder.java:224)
        at brut.androlib.res.decoder.ARSCDecoder.readTablePackage(ARSCDecoder.java:133)
        at brut.androlib.res.decoder.ARSCDecoder.readTableHeader(ARSCDecoder.java:85)
        at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:51)
        ... 8 more

Steps to Reproduce

apktool w.apk

APK

If this APK can be freely shared, please upload/attach a link to it. https://www.apkmirror.com/apk/whatsapp-inc/whatsapp/whatsapp-2-23-8-4-release/

Brad1944 commented 1 year ago

Hi, sorry for bumping, but is there any update on this issue?

teamb58 commented 1 year ago

Yes, @iBotPeaches any update on this? Looks like all Meta apps are affected.

It was working until a few versions back. I would request you to have a look at this one please.

Thanks.

tmdh commented 1 year ago

@iBotPeaches After a little investigation I found that:

ARSCDecoder expects a short "8" here. But according to android-arscblamer, this short can be anything.
It also expects a "0" byte after the size here. But in android-arscblamer, the byte is skipped.

So I changed the function readValue like this:

private ResIntBasedValue readValue() throws IOException, AndrolibException {
-               mIn.skipCheckShort((short) 8); // size
-               mIn.skipCheckByte((byte) 0); // zero
+               mIn.skipBytes(2);
+               mIn.skipBytes(1);
         byte type = mIn.readByte();
         int data = mIn.readInt();

         return type == TypedValue.TYPE_STRING
                 ? mPkg.getValueFactory().factory(mTableStrings.getHTML(data), data)
                 : mPkg.getValueFactory().factory(type, data, null);
}

I am still getting more errors:

Exception in thread "main" brut.androlib.exceptions.AndrolibException: Invalid value type: -1
        at brut.androlib.res.data.value.ResValueFactory.factory(ResValueFactory.java:67)
        at brut.androlib.res.decoder.ARSCDecoder.readValue(ARSCDecoder.java:408)
        at brut.androlib.res.decoder.ARSCDecoder.readEntryData(ARSCDecoder.java:325)
        at brut.androlib.res.decoder.ARSCDecoder.readTableType(ARSCDecoder.java:309)
        at brut.androlib.res.decoder.ARSCDecoder.readTableTypeSpec(ARSCDecoder.java:224)
        at brut.androlib.res.decoder.ARSCDecoder.readTablePackage(ARSCDecoder.java:132)
        at brut.androlib.res.decoder.ARSCDecoder.readTableHeader(ARSCDecoder.java:84)
        at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:50)
        at brut.androlib.res.AndrolibResources.getResPackagesFromApk(AndrolibResources.java:816)
        at brut.androlib.res.AndrolibResources.loadMainPkg(AndrolibResources.java:99)
        at brut.androlib.res.AndrolibResources.getResTable(AndrolibResources.java:91)
        at brut.androlib.ApkDecoder.getResTable(ApkDecoder.java:166)
        at brut.androlib.ApkDecoder.decodeManifest(ApkDecoder.java:289)
        at brut.androlib.ApkDecoder.decode(ApkDecoder.java:104)
        at brut.apktool.Main.cmdDecode(Main.java:184)
        at brut.apktool.Main.main(Main.java:88)

Just wanted to share what I found.

iBotPeaches commented 1 year ago

Thanks @tmdh - The only change I see there that we could make better is the assumption of the data size (8). You can see here the spec says the byte must be zero - https://github.com/iBotPeaches/platform_frameworks_base/blob/master/libs/androidfw/include/androidfw/ResourceTypes.h#L288

However, it does appear that its still short/byte/byte/int for size/reserved/type/value. I think a patch we could experiment with is checking if we read all the data from the reported size. The reason we do a read check on the first short is because we only read 8 bytes of data. If the size wasn't 8 - it would crash anyway.

iBotPeaches commented 1 year ago

Okay I spent some time today investigating this and what appears to be happening is the reported size of a chunk is wrong. In this example we are finding that the chunks we are reading are exceeding the size that was reported of that chunk.

I always figured it was best to trust the disassembly and follow the spec - sometimes skipping bytes when we don't have enough data to fill that chunk. In this example the opposite is occurring. We are being told we have X bytes of data and reading way more than that.

However, where this is odd is that the data being read appears as partially valid items in that chunk. For example, lets say we are reading 82 entries of resources. We may discover that there is actually 112 in that set. We read them all, but they aren't exactly valid resources (maybe a few are) and that leads to that crash above.

Apktool unfortunately doesn't have the best logic to discover how much we've read/processed per chunk to properly skip the what I'm assuming is bogus data. Nor do I want to write something that just skips data unless I'm fairly confident its intentional bogus data to break tools.

So tldr - more work needed.

iBotPeaches commented 1 year ago

Merged a large refactor of AXML/ARSC parsing this morning - https://github.com/iBotPeaches/Apktool/commit/bdbe1384bf037a32b9af8258cb6bab09bbce7b21. This opens up the possibility to have control over the chunk reading a bit easier to make this ticket doable.

Ran out of time this morning, so just leaving a few of my notes.

0x155228 - chunkStart
0x1588E4 - chunkEnd
0x36BC - chunkSize
0x15887C - start of element prior to crash
0x1588EC - right before crash

iBotPeaches commented 1 year ago

Got a PR up, will close this when its merged. See the PR for the details - it does confuse me.

I: Using Apktool 2.7.1-863405-SNAPSHOT on whatsapp.apk
I: Loading resource table...
W: End of chunk hit. Skipping remaining entries (1763) in type: attr
I: Decoding AndroidManifest.xml with resources...
I: Loading resource table from file: /home/ibotpeaches/.local/share/apktool/framework/1.apk
I: Regular manifest package...
I: Decoding file-resources...
I: Decoding values */* XMLs...
I: Copying raw classes.dex file...
I: Copying raw classes2.dex file...
I: Copying raw classes3.dex file...
I: Copying raw classes4.dex file...
I: Copying assets and libs...
I: Copying unknown files...
I: Copying original files...
I: Copying META-INF/services directory
➜  3036

iBotPeaches / Apktool