[BUG] `apktool b` generates bad `hiddenapi_class_data_item`

[RenateUSB]

Information

1. Apktool Version 2.6.1
2. Windows 10
3. Onyx Poke3 framework.jar, but narrowed down some

The original problem was that apktool d -api 29, apktool b -api 29 succeeded but the resulting jar did not work.
The deeper dive of apktool d -api 29, apktool b -api 29, apktool d -api 29 gave the below error.
I then did a fresh apktool d -api 29 and tested it with apktool b -api 29, apktool d -api 29.
I deleted classes as much as I could until the apktool b -api 29, apktool d -api 29 returned no error, then backed up.
At this point it consisted of only two smali classes that still gave the same error.
Deleting various virtual methods from ProtoOutputStream might cause the error to go away, or not.
I also cleaned all greylist-max-? to simple greylist
Various versions caused the OOB to be 7 instead of 6.

1. baksmali is using an array of length 6 to be indexed by 3 bits (masked by 0x7), a recipe for disaster.
2. apktool is still using baksmali that does not support greylist-max-r. (This example never used greylist-max-r.)
3. Even the hidden API flags that it did decode are all wrong, many greylist were changed to whitelist. writeRepeatedFixed32Impl(II)V even changed from greylist to blacklist.

Stacktrace/Logcat

Error occurred while disassembling class Landroid.util.proto.ProtoOutputStream; - skipping class
java.lang.ArrayIndexOutOfBoundsException: 6
        at org.jf.dexlib2.HiddenApiRestriction.getAllFlags(HiddenApiRestriction.java:108)
        at org.jf.dexlib2.dexbacked.DexBackedMethod.getHiddenApiRestrictions(DexBackedMethod.java:204)

Steps to Reproduce

1. unzip fw.zip
2. apktool b FW0 -o fw1.jar -api 29
3. apktool d fw1.jar -o FW1 -api 29

fw.zip

[RenateUSB]

Ok, here is a further case, similar to the preceeding one.
It does not generate the error of the preceeding one but it does screw up the hidden API markers.
This example still has the problematic ProtoStream class, ProtoOutputStream was removed.
It has a demo class of HapiObject (super java.lang.object) and HapiProto (super android.util.proto.ProtoStream).
HapiObject is happy with the hidden API markers propagated.
HapiProto has all the markers changed to greylist
ProtoStream, as before, has changes to the markers.
This is (part of) the original smali code of both Hapi files.

.field public whitelist HAPI0_whitelist:I

.field public greylist HAPI1_greylist:I

.field public blacklist HAPI2_blacklist:I

.field public greylist-max-o HAPI3_greylist_max_o:I

.field public greylist-max-p HAPI4_greylist_max_p:I

.field public greylist-max-q HAPI5_greylist_max_q:I

Steps to Reproduce

1. unzip hapi.zip
2. apktool b FW0 -o fw1.jar -api 29
3. apktool d fw1.jar -o FW1 -api 29
4. Examine FW1\smali\android\util\HapiProto.smali

hapi.zip

[RenateUSB]

Ok, we're narrowing it down!
The problem is if a class has a super with a greylist constructor.
We're even back to getting that array OOB error.
ProtoStream has been pruned down to nothing.

.class public abstract Landroid/util/proto/ProtoStream;
.super Ljava/lang/Object;
.source "ProtoStream.java"


# direct methods

.method public constructor greylist <init>()V
    .locals 0

    .prologue
    invoke-direct {p0}, Ljava/lang/Object;-><init>()V

    return-void
.end method

Steps to Reproduce

1. unzip min.zip
2. apktool b FW0 -o fw1.jar -api 29
3. apktool d fw1.jar -o FW1 -api 29
4. Generates array OOB and hidden API markers in HapiProto are messed up. Compare to HapiObject.

min.zip

Edit: Changing the greylist to whitelist in ProtoStream still breaks things, but changes the fields that it can decode in HapiProto to whitelist.
Getting rid of the marker in ProtoStream removes all markers from HapiProto and produces no errors.

[RenateUSB]

Apparently a fix was merged??? JesusFreke/smali#816
But the commit is not part of the git??? JesusFreke/smali@3436508
See: #2662

[iBotPeaches]

@RenateUSB - Just looks like it was rebased/squashed - JesusFreke/smali@81bd303

@JesusFreke - You have a release planned soon for some of these merged, but unreleased fixes?

[RenateUSB]

Just to beat a dead horse...
In a dex file the classes must always have any super before a subclass.
So in that "min.zip" example above, the class order was:

HapiObject
ProtoStream
HapiProto

In the hiddenapi_class_data_item the hidden API flags are concatenated in order of class,
Unfortunately the current (incorrect) order that is being used is alphabetic.
In decoding the dex it's trying to decode the third set of flags (since HapiProto is last) and grabbing the flags for ProtoStream.
Moreover, since ProtoStream has less flags it's actually reading past the end of the valid flags.
It's grabbing a random 0x0e which when masked with 0x07 gives you the spurious 6 that is reported as error.
The fix?

HapiObject
ProtoStream
ZapiProto

This works fine. Now alphabetical order and class order match.
Ok, renaming classes might break something later...

This is quite a serious bug. It means rebuilding any api>=29 Android core file has about zero chance of being correct even if no error is generated.

[iBotPeaches]

I believe fixed with: #2941

[r6680jc]

@RenateUSB

Have you tested apktool 2.7.0 to decompile and recompile framework.jar?

In my case, this issue still exists (same as self build smali/baksmali).

[RenateUSB]

@r6680jc No, not yet, but I'll check it out.
I have grown fond of directly patching 4 bytes in 30 M though.
#2947

Edit: I can confirm that the hidden API data appears to be correct in 15,000 classes.
Unfortunately, a round-trip of framework.jar causes my Android to be a train wreck.
A round-trip changes the order of classes so it's not easy to compare.
Still investigating...

Trivia: What class has the most fields & methods.
android.util.StatsLogInternal has 4500? WTH?

[r6680jc]

@RenateUSB

It's just the resulting framework.jar with any classes.dex rebuilt without modification makes the device bootloop.