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?