Change deobf whitelist behavior

[bagipro]

This PR changes the behavior of the --deobf-whitelist arg:

1. I haven't found any references to why this option has predefined values. As a researcher, I can say that there is no practical reason to leave the androidx.* or android.support.* libraries undeobfuscated. If it is a non-obfuscated application, it will not be changed anyway. However, if some crazy obfuscation is applied, it might get moved to weird packages like p000. So I removed the defaults
2. Now the behavior is applied to all members of the provided whitelist pattern, including child packages, member fields and methods

[nitram84]

Hi @bagipro , I proposed this whitelist because deobfuscating "v4" to "p000v4" when it is part of a package name "android.support.v4.*" is in nearly every case a false positive. Before adding this whitelist these false positives could even be found when you tried to deofuscate an non-obfuscated app.
Trying to minimize those false positives using a whitelist allows you to perform a dependency detection (see #2040). Using a whitelist also makes recompiling easier because support libraries can be replaced with directly the dependency without renaming "p000v4"-false positives.

Do you have any example where this whitelist introduces new false positives? If so, I think the better way is to detect and prevent the false positives.

[bagipro]

Hey @nitram84,

Ahh yes. I've always used jadx with --deobf-min set to 2 instead of the default 3 (because it fixed a lot of false positives in common short package names like ui or the cases you describe). That's why I never experienced these problems.

Do you have any example where this whitelist introduces new false positives?

My idea was to make this feature consistent, i.e. it should consider class content, not just class and package names.

I think the best way to cover all cases would be to change --deobf-min to 2 by default. On the one hand, this would allow whitelisting the entire package or class content, and on the other hand, it would avoid all the discussed false positives. What do you think?

[bagipro]

Hey @skylot and @nitram84,

I've changed the --deobf-min to 2 in the recent commit. What do you think about this approach?

[nitram84]

Hi @bagipro,

I understand. You are right, with --deobf-min set to 2 a whitelist is not required.

But I'm not sure if --deobf-min =2 is good default for newer apps with more aggressive obfuscation techniques. With more than 26 classes in package and more than 26 members per class and every name in lower case the current default --deobf-min =3 might be a good value.

Also the whitelist is currently not the only feature that relies only on class and package names without considering the class content. We have already

• a non optional whitelist for TLDs: https://github.com/skylot/jadx/blob/master/jadx-core/src/main/resources/jadx/core/deobf/conditions/tld_3.txt and
• a heuristic for R files.

With --deobf-min < 3 whitelist and TLDs could be ignored (minor optimization) but for --deobf-min >= 3 the whitelist is still helpful to reduce false positives in my opinion. The whitelist was not complete: a missing package was e.g. kotlin.js.* see https://github.com/niranjan94/show-java/releases/download/v3.0.5/ShowJava_v3_0_5.apk .

If it is possible I would like to keep the whitelist for --deobf-min >= 3.

[bagipro]

Hey!

With more than 26 classes in package

It should be 26 * 26 (2 characters) or 35 * 35 (2 characters when including numbers). Additionally, jadx avoids duplicated class/package names (it's applicable for obfuscated apps) and name collisions for case-insensitive file systems, so it shouldn't be a problem.

If it is possible I would like to keep the whitelist for --deobf-min >= 3.

But do you think it would be a problem when the default value is 2? I mean that if someone changes it to higher numbers, let's say 4, many more classes than the default whitelist will be affected.

Thanks!

[skylot]

I would like to keep the whitelist for --deobf-min >= 3

I also think that changing deobf-min default value can be confusing for some people 🙂
As for deobf whilelist: its main purpose is to keep names regardless of current deobfuscation settings and current android.support.* and androidx.* are quite sane defaults. But I agree that current implementation is only whitelist exact package and its classes without subpackages and methods/fields in classes. Such implementation is misleading and should be fixed 👍

@bagipro I think I can slightly improve performance of your check for subpackages (match over stream). I will apply these changes with undo of default values changes.

[bagipro]

@skylot
That's cool!

But a bug might happen if we keep the default whitelist, keep --deobf-min set to 3 and also change the behavior to keep all whitelist members unchanged, because most of the applications are obfuscated and it might produce tons of compilation errors when using the default configuration.

What do you think about this? That's why my idea was to use 2 of these workarounds.

[skylot]

might produce tons of compilation errors

Well, deobfuscation isn't supposed to fix compilation errors, we have other checks for that. Ideally, jadx should output correct code even with disabled deobfuscation 🙂
Can you create a new issue for such errors, so I can improve these checks?

[skylot]

I applied my changes: exclude list calculated in init method using internal packages tree, so implementation is very simple now. This is also forced me to fix init order of rename providers, because previously package info was not yet set.

@bagipro, @nitram84 please check my changes, I still not sure about default packages for exclude, so suggestions are appreciated.

[nitram84]

Hi @skylot,

thank you for your work. I tested the PR with a apk having obfuscated classes in a package on the whitelist: https://apkpure.com/database-modeler-lite/adrian.adbm/download/2.2
I my opinion there is no need to exclude obfuscated classes in androidx.core.os.* from deobfuscation. For me the minimal requirement is to keep the package name for all classes in androidx.core.os.

My initial idea of #2040 was a whitelist to exclude specific classes and package names (keep the package name for all classes inside the package) from deobfuscation. So I tried to keep the whitelist as restrictive as possible. This is list I am using currently:

android.support.v4.* android.support.v7.* android.support.v4.os.* android.support.annotation.Px androidx.core.os.* androidx.annotation.Px kotlin.js.*

The entry kotlin.js.* is new on the list (see https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.js/).

I don't think the list is complete. I think a lot of third party package are missing, but I haven't them yet because I'm replacing sources of other libs directly with the gradle dependency when I recompile apps 🙂

My intention was to use deobfuscation as an always-enabled feature (even for non-obfuscated or unknown apps) without having any drawbacks.

[skylot]

apk having obfuscated classes in a package on the whitelist

I think it is not possible to create a list which will work for all apps, because app classes can be moved to any package.
I don't have a stats but looks like most apps don't put own classes in android or androidx packages, so such defaults is quite sane to me. And if app is an exception, user can adjust the exclude list (now excluded classes have jadx info comment, so it should be clear why it is not renamed).

@nitram84 I understand that you want to pinpoint only some packages like v4, os, etc, but recursive approach suggested by @bagipro in this PR is made this task harder. Also, such strict rules depend on deobf-min length option, so it is very specific and can be not helpful if user change this option.

Some possible improvements:

• if only package names need to be excluded, we can search entries without '.*' also in packages list and exclude only that package.
• for jadx-gui we can simplify edit this list by per package/class popup menu entry like 'Exclude from deobfuscation' similar to current exclude from decomplation action.
• another improvement for jadx-gui can be: make this option saved by per project settings, so it will be posible to change it only for current apk.

Anyway, deobfuscation rules/patterns is a very debatable topic and to allow customization I added scripting support to jadx, so anyone can adjust deobfuscation with scripts now 🤣