Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Newer Jadx versions generate random alphanumeric entries breaking the original code reconstruction #1672

Closed
JohnFarl opened this issue Sep 18, 2022 · 10 comments
Assignees
Labels
bug Core Issues in jadx-core module rename
Milestone

Comments

@JohnFarl
Copy link

JohnFarl commented Sep 18, 2022

The code decompiled with the newer version seems worse than the code decompiled with an old Jadx version.

There are a lot of lines broken with random codes everywhere e.g.

  • CollapsingToolbarLayout.java

Latest Jadx (line 22)
import android.support.p001v4.content.ContextCompat;
Old Jadx (line 22)
import android.support.v4.content.ContextCompat;

Latest Jadx (line 84)
TypedArray a = context.obtainStyledAttributes(attrs, C0016R.styleable.CollapsingToolbarLayout, defStyleAttr, C0016R.style.Widget_Design_CollapsingToolbar);
Old Jadx (line 84)
TypedArray a = context.obtainStyledAttributes(attrs, R.styleable.CollapsingToolbarLayout, defStyleAttr, R.style.Widget_Design_CollapsingToolbar);

  • SocketNode.java

Latest Jadx (line 61)

Logger mo1619getLogger = this.context.mo1619getLogger(iLoggingEvent.getLoggerName());
                if (mo1619getLogger.isEnabledFor(iLoggingEvent.getLevel())) {
                    mo1619getLogger.callAppenders(iLoggingEvent);
                }

Older Jadx (line 61)

Logger logger = this.context.getLogger(iLoggingEvent.getLoggerName());
                if (logger.isEnabledFor(iLoggingEvent.getLevel())) {
                    logger.callAppenders(iLoggingEvent);
                }

There are countless similar cases, and changing the decompilation mode from AUTO to RESTRUCTURE leads to analogous result.

The apk I have tried link.

Unfortunately, I cannot say what is the old version I'm talking about, because I have updated it many time without paying attention to this detail.

@JohnFarl JohnFarl added bug Core Issues in jadx-core module labels Sep 18, 2022
@jpstotz
Copy link
Collaborator

jpstotz commented Sep 18, 2022

Have you tried to disable deobfuscation? The package name v4 is very short and thus Jadx assumes that it is an obfuscated package name and uses the generated package name instead.

@JohnFarl
Copy link
Author

@jpstotz I haven't tried. I will try but similar things are generated also with packages with longer names and simple code sections, not only with v4. (As you can see even in the mess of latest sample with logger piece of code. )

@JohnFarl
Copy link
Author

JohnFarl commented Sep 18, 2022

UPDATE
@jpstotz
If I try to disable the deobfuscation as suggested the problem mentioned is significantly reduced but still remains

Example

  • Fade.java

Latest Jadx version (line 52)
mo43addListener(new TransitionListenerAdapter() { // from class: android.support.transition.Fade.1

Old Jadx version (line 52)
addListener(new TransitionListenerAdapter() { // from class: android.support.transition.Fade.1

  • Visibility.java

Latest Jadx version (line 254)
mo43addListener(disappearListener);

Old Jadx version (line 254)
addListener(disappearListener);

@jpstotz
Copy link
Collaborator

jpstotz commented Sep 18, 2022

The getLogger method is a different case. I would assume that Jadx renames this method because it has detected a method name collision.

In detail method ch.qos.logback.classic.LoggerContext.getLogger(java.lang.String) is renamed to mo175getLogger by the last block of jadx.core.dex.visitors.rename.RenameVisitor#checkMethods.

The two conflicting methods are:

  • ch.qos.logback.classic.LoggerContext.getLogger(java.lang.String):ch.qos.logback.classic.Logger
  • ch.qos.logback.classic.LoggerContext.getLogger(java.lang.String):org.slf4j.Logger

The interesting part is that the second method is marked as bridge synthetic in Smali but I am still not sure what this means on Java level as to my knowledge it should not be possible to have two methods that only differ regarding their return type (even if one is a synthetic method - or is this different on DEX/Smali level? As on Smali all methods are referenced by their full method signature including return type it may be possible to have overloaded methods that only differ regarding the return type).

@skylot I first debugged a different part of Jadx when searching for the cause. I started with checkMethodSignatureCollisions and noticed that Jadx claims to detect multiple collisions on the class itself which doesn't seem correct to me.

For example using the sample APK file FIMI Navi_vV1.0.20_apkpure.com.apk linked in the first post of this issue Jadx identifies a collision on createFromParcel(Landroid/os/Parcel;) in the class android.support.design.internal.BottomNavigationPresenter.SavedState.1 with itself and thus renames it.

@skylot
Copy link
Owner

skylot commented Sep 18, 2022

@jpstotz I see, jadx renames these methods due to collision with bridge method, but next remove/merge that bridge method, so this renames not needed anymore. I will try to fix this.

@JohnFarl as workaround for now, you can disable all checkboxes in Rename identifiers group in preferences to completely disable all renaming.

@jpstotz

to have two methods that only differ regarding their return type

yes, this allowed in bytecode, so unique method signature contains return type

@skylot skylot self-assigned this Sep 18, 2022
@skylot skylot added this to the TBD milestone Sep 18, 2022
@skylot skylot added the rename label Sep 18, 2022
@JohnFarl
Copy link
Author

JohnFarl commented Sep 18, 2022

@jpstotz @skylot Disabling the rename options I get more consistent code.
However I think these collision should be managed with proper packaging structure reconstruction.

A code with colliding methods cannot be compiled in first place, so if Jadx produces colliding code that needs to refactor to avoid collisions, this certainly happens because it merges code from different packages that isn't supposed to be in same place.

@jpstotz
Copy link
Collaborator

jpstotz commented Sep 18, 2022

A code with colliding method cannot be compiled in first place, so if Jadx produces colliding code that has to refactor to avoid collision, this certainly happens because it merges code from different packages that isn't supposed to be in same place.

Not in this case. The colliding code is generated by the Android dex compiler may be to apply some optimization or whatever. So the original Java code was free of method collisions and also the generated class files. But when the APK files was created and everything was compiled/translated to DEX code the synthetic methods were added and thus the method collisions were introduced.

@JohnFarl
Copy link
Author

@jpstotz How a colliding code is supposed to be executed without crashing at runtime due to ambigous method signatures? Because they are in different bytecode blocks that reside in different sections and are called by different methods, so it doesn't make difference for the executor if there are methods with same signatures, because these are called separately, this is a problem only in compilation time, but if these blocks are decompiled in order to not have visibility outside the respective invokers a refactor isn't needed.
(Also if the colliding methods are actually the same, the method could be decompiled once uniforming invocations rather than having repeated refactored methods. )

@jpstotz
Copy link
Collaborator

jpstotz commented Sep 18, 2022

@jpstotz How a colliding code is supposed to be executed without crashing at runtime due to ambigous method signatures?

Dex is more powerful than Java class files. On dex level methods are referenced by a signature that includes the return type. Therefore for Android there are no ambigous methods.

@skylot
Copy link
Owner

skylot commented Sep 23, 2022

Fixed.
Although this fix is specific for this case, i.e. bridged overridden methods. And looks like this is a most common source of bridged methods generated by javac as described in java doc. If anyone found any other cases, please report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Core Issues in jadx-core module rename
Projects
None yet
Development

No branches or pull requests

3 participants