skylot / jadx

Dex to Java decompiler
Apache License 2.0
41.04k stars 4.81k forks source link

[feature] Support switch over strings #2278

Open Real-Packet opened 2 days ago

Real-Packet commented 2 days ago

Describe your idea

How I found this

Hello, it seems like when decompiling Gryphon Connect version 05.0003.40 from APKPure, the following switch case is present in com.gryphonconnect.gryphon.utils.devicelists.DeviceUtils in the getDeviceImage function. I decompiled it using APKLab, which uses jadx for decompiling. Here's the APK (it's in the ZIP file the APK is 30 MB, zipping it gives you 25.4 MB, and GitHub doesn't allow more than 26 MB or so): Gryphon Connect_05.0003.40.zip

APKTool.yml file version: 2.9.3 apkFileName: Gryphon Connect_05.0003.40_APKPure.apk isFrameworkApk: false usesFramework: ids: - 1 tag: null sdkInfo: minSdkVersion: 21 targetSdkVersion: 33 packageInfo: forcedPackageId: 127 renameManifestPackage: null versionInfo: versionCode: 147 versionName: 05.0003.40 resourcesAreCompressed: false sharedLibrary: false sparseResources: false unknownFiles: DebugProbesKt.bin: 8 androidsupportmultidexversion.txt: 0 billing.properties: 8 en-mnemonic-word-list.txt: 8 firebase-analytics.properties: 8 firebase-annotations.properties: 8 firebase-components.properties: 8 firebase-datatransport.properties: 8 firebase-encoders-json.properties: 8 firebase-encoders-proto.properties: 8 firebase-encoders.properties: 8 firebase-iid-interop.properties: 8 firebase-installations-interop.properties: 8 firebase-measurement-connector.properties: 8 messaging_event.proto: 8 messaging_event_extension.proto: 8 play-services-ads-identifier.properties: 8 play-services-base.properties: 8 play-services-basement.properties: 8 play-services-cloud-messaging.properties: 8 play-services-location.properties: 8 play-services-maps.properties: 8 play-services-measurement-api.properties: 8 play-services-measurement-base.properties: 8 play-services-measurement-impl.properties: 8 play-services-measurement-sdk-api.properties: 8 play-services-measurement-sdk.properties: 8 play-services-measurement.properties: 8 play-services-stats.properties: 8 play-services-tasks.properties: 8 transport-api.properties: 8 transport-backend-cct.properties: 8 transport-runtime.properties: 8 web3j-version.properties: 8 stamp-cert-sha256: 8 jni/Darwin/libjffi-1.2.jnilib: 8 jni/i386-Windows/jffi-1.2.dll: 8 jni/ppc-AIX/libjffi-1.2.a: 8 jni/ppc64-AIX/libjffi-1.2.a: 8 jni/x86_64-Windows/jffi-1.2.dll: 8 okhttp3/internal/publicsuffix/NOTICE: 8 okhttp3/internal/publicsuffix/publicsuffixes.gz: 0 org/bouncycastle/x509/CertPathReviewerMessages.properties: 8 org/bouncycastle/x509/CertPathReviewerMessages_de.properties: 8 solidity/build.sh: 8 solidity/readme.txt: 8 solidity/ens/AbstractENS.sol: 8 solidity/ens/ENS.sol: 8 solidity/ens/PublicResolver.sol: 8 solidity/ens/build/AbstractENS.abi: 8 solidity/ens/build/AbstractENS.bin: 0 solidity/ens/build/ENS.abi: 8 solidity/ens/build/ENS.bin: 8 solidity/ens/build/PublicResolver.abi: 8 solidity/ens/build/PublicResolver.bin: 8 doNotCompress: - resources.arsc - META-INF/androidx.activity_activity.version - META-INF/androidx.annotation_annotation-experimental.version - META-INF/androidx.appcompat_appcompat-resources.version - META-INF/androidx.appcompat_appcompat.version - META-INF/androidx.asynclayoutinflater_asynclayoutinflater.version - META-INF/androidx.cardview_cardview.version - META-INF/androidx.coordinatorlayout_coordinatorlayout.version - META-INF/androidx.core_core-ktx.version - META-INF/androidx.core_core.version - META-INF/androidx.cursoradapter_cursoradapter.version - META-INF/androidx.customview_customview.version - META-INF/androidx.documentfile_documentfile.version - META-INF/androidx.drawerlayout_drawerlayout.version - META-INF/androidx.dynamicanimation_dynamicanimation.version - META-INF/androidx.emoji2_emoji2-views-helper.version - META-INF/androidx.emoji2_emoji2.version - META-INF/androidx.exifinterface_exifinterface.version - META-INF/androidx.fragment_fragment.version - META-INF/androidx.interpolator_interpolator.version - META-INF/androidx.legacy_legacy-support-core-ui.version - META-INF/androidx.legacy_legacy-support-core-utils.version - META-INF/androidx.legacy_legacy-support-v4.version - META-INF/androidx.loader_loader.version - META-INF/androidx.localbroadcastmanager_localbroadcastmanager.version - META-INF/androidx.media_media.version - META-INF/androidx.print_print.version - META-INF/androidx.profileinstaller_profileinstaller.version - META-INF/androidx.recyclerview_recyclerview.version - META-INF/androidx.savedstate_savedstate.version - META-INF/androidx.slidingpanelayout_slidingpanelayout.version - META-INF/androidx.startup_startup-runtime.version - META-INF/androidx.swiperefreshlayout_swiperefreshlayout.version - META-INF/androidx.tracing_tracing.version - META-INF/androidx.transition_transition.version - META-INF/androidx.vectordrawable_vectordrawable-animated.version - META-INF/androidx.vectordrawable_vectordrawable.version - META-INF/androidx.versionedparcelable_versionedparcelable.version - META-INF/androidx.viewpager2_viewpager2.version - META-INF/androidx.viewpager_viewpager.version - META-INF/com.google.android.material_material.version - META-INF/kotlinx_coroutines_android.version - META-INF/kotlinx_coroutines_core.version - META-INF/services/com.fasterxml.jackson.core.JsonFactory - META-INF/services/com.fasterxml.jackson.core.ObjectCodec - mp3 - assets/dexopt/baseline.prof - assets/dexopt/baseline.profm - png - gif - ogg

The pattern I'm seeing

because the switch statement is big, I'm going to spare you some scrolling, here's what the pattern looks like:

switch (what.hashCode()) {
    case 1234:
      // this is probably because of hashCode collision, so we make sure it's not some colliding thing.
      if (what.equals("whatever it seems like it is supposed to be")) {
           // do whatever, it is whatever it seems like it supposed to be
          break;
      }
      break;
    // ...
}

How you can reverse this

Checklist for reversing the hash-code optimization(?):

When you handle a switch statement case, check if what we're switching based on a hashCode. If we are switching based on a hashCode, see if there is any comparison to what we're switching on.

Psuedo-code for reversing the optimization

Note that this doesn't handle multi-string cases, something like this (based on the pattern):

switch (what.hashCode()) {
    case 1:
    case 2:
    case 3:
      // this is probably because of hashCode collision, so we make sure it's not some colliding thing.
      // #1
      if (what.equals("whatever it seems like it is supposed to be")) {
           // do whatever, it is whatever it seems like it supposed to be
          break;
      }
      // #2
      if (what.equals("whatever it seems like it is supposed to be #2")) {
           // do whatever, it is whatever it seems like it supposed to be #2
          break;
      }
      // #3
      if (what.equals("whatever it seems like it is supposed to be #3")) {
           // do whatever, it is whatever it seems like it supposed to be #3
          break;
      }
      break;
    // ...
}
public String getStringFromHashCodeOptimization(SwitchCaseNode node) {
    IfStatementNodes[] ifNodes = node.childNodes.filterForIfStatements();
    for (ifNode of ifNodes) {
         // node.switch.comparison: a.hashCode()
         // node.switch.comparison.invoked.parent: a
         // ifNode.comparison: a.equals("{something}")
         // ifNode.comparison.invoked.parent: a
         // ifNode.comparison.invoked.arguments: ["{something}"]
         if (node.switch.comparison.invoked.parent.equals(ifNode.comparison.invoked.parent)) {
               // found it, get the string in the comparison.
               return ifNode.comparison.invoked.arguments[0];
         }
    }
}

The raw switch statement

So you can see for yourself without decompiling it with jadx, here's what it looks like.

switch (str.hashCode()) {
            case -1841265815:
                if (str.equals("Router")) {
                    c = 0;
                    break;
                }
                break;
            case -1797510522:
                if (str.equals("Tablet")) {
                    c = 1;
                    break;
                }
                break;
            case -1707937765:
                if (str.equals("Webcam")) {
                    c = 2;
                    break;
                }
                break;
            case -1678803657:
                if (str.equals("Console")) {
                    c = 3;
                    break;
                }
                break;
            case -1365917577:
                if (str.equals("Thermostat")) {
                    c = 4;
                    break;
                }
                break;
            /// ... (I'm not providing the entire thing because you get the point)
}
jackwpa commented 2 days ago

It looks like this is what JEB Pro is doing, essentially a double-switch reconstruction (switch on String.hashCode+equals and assigning a small index to reduce the selection space, then switch on that index):

image

Real-Packet commented 2 days ago

Also, this happens on jadx-gui (the flatpak version) too, it's not just an APKLab or APKTool thing. image

Real-Packet commented 2 days ago

The following ZIP file has the APK that was mentioned in the issue. I'm going to update the issue comment to add this. Gryphon Connect_05.0003.40.zip

Real-Packet commented 2 days ago

I'm going to open a PR for this, as I don want to wait if I can just do it myself.

Real-Packet commented 2 days ago

I am working on the code. Currently looking at RegionGen#makeSwitch because that's where the cases are handled.

Real-Packet commented 2 days ago

I'm gonna use JetBrains IDEA because VSCode seems to be assuming that RegionGen.java is a non-project file.

Real-Packet commented 2 days ago

Well, time to install Java 11. image

Real-Packet commented 2 days ago

Windows users when they have to install Java: java.com, download installer, run installer (winget is mid) Linux users: sudo {whateverPackageManagerOutOfThe5ThousandPackageManagersYouUse} install java-{version}openjdk

Real-Packet commented 2 days ago

image I can build jadx-gui now, although I did have to enable a experimental feature thing in the JetBrains IDEA settings (which of course binds me to another terms of service for experimental settings, but I don't care).

Real-Packet commented 2 days ago

Also, dark mode when? (lol)

Real-Packet commented 2 days ago

image Ok, so I can decompile it. This means my build isn't faulty (at least, I think), so I guess we can continue on to debugging.

skylot commented 1 day ago

Also, dark mode when? (lol)

@Real-Packet you can change UI and editor themes in Preferences -> Appearance. And it may be helpful to set default themes according used in user system, with lib like jSystemThemeDetector it will be not hard. I will create a new feature issue for that :slightly_smiling_face:

Real-Packet commented 1 day ago

When I set breakpoints and run the GUI, it never hits them. Moment.

skylot commented 1 day ago

When I set breakpoints and run the GUI, it never hits them

Decompilation results cached by default, you can disable caching by setting Preferences->Cache->Code cache mode to Memory. Also, you can use Tools->Reset code cache menu item. But better to use integration test like this:

package jadx.tests.integration.switches;

import jadx.tests.api.IntegrationTest;
import org.junit.jupiter.api.Test;

import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;

public class TestSwitchOverStrings extends IntegrationTest {

    /**
     * Strings 'frewhyh', 'phgafkp' and 'ucguedt' have same hash code.
     */
    public static class TestCls {

        public int test(String str) {
            switch (str) {
                case "frewhyh":
                    return 1;
                case "phgafkp":
                    return 2;
                case "test":
                    return 3;
                case "other":
                    return 4;
                default:
                    return 0;
            }
        }

        public void check() {
            assertThat(test("frewhyh")).isEqualTo(1);
            assertThat(test("phgafkp")).isEqualTo(2);
            assertThat(test("test")).isEqualTo(3);
            assertThat(test("other")).isEqualTo(4);
            assertThat(test("unknown")).isEqualTo(0);
            assertThat(test("ucguedt")).isEqualTo(0);
        }
    }

    @Test
    public void test() {
        assertThat(getClassNode(TestCls.class))
                .code()
                .doesNotContain("case -603257287:")
                .containsOne("case \"frewhyh\":");
    }
}

Just paste this code into jadx-core/src/test/java/jadx/tests/integration/switches and run

Real-Packet commented 1 day ago

When I set breakpoints and run the GUI, it never hits them. Moment.

Ok, after resetting the cache, it works.

Real-Packet commented 1 day ago

When I set breakpoints and run the GUI, it never hits them

Decompilation results cached by default, you can disable caching by setting Preferences->Cache->Code cache mode to Memory. Also, you can use Tools->Reset code cache menu item. But better to use integration test like this:

package jadx.tests.integration.switches;

import jadx.tests.api.IntegrationTest;
import org.junit.jupiter.api.Test;

import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;

public class TestSwitchOverStrings extends IntegrationTest {

  /**
   * Strings 'frewhyh', 'phgafkp' and 'ucguedt' have same hash code.
   */
  public static class TestCls {

      public int test(String str) {
          switch (str) {
              case "frewhyh":
                  return 1;
              case "phgafkp":
                  return 2;
              case "test":
                  return 3;
              case "other":
                  return 4;
              default:
                  return 0;
          }
      }

      public void check() {
          assertThat(test("frewhyh")).isEqualTo(1);
          assertThat(test("phgafkp")).isEqualTo(2);
          assertThat(test("test")).isEqualTo(3);
          assertThat(test("other")).isEqualTo(4);
          assertThat(test("unknown")).isEqualTo(0);
          assertThat(test("ucguedt")).isEqualTo(0);
      }
  }

  @Test
  public void test() {
      assertThat(getClassNode(TestCls.class))
              .code()
              .doesNotContain("case -603257287:")
              .containsOne("case \"frewhyh\":");
  }
}

Just paste this code into jadx-core/src/test/java/jadx/tests/integration/switches and run

Also, I will add the unit tests, later :tm:

Real-Packet commented 1 day ago

Ok, I've basically gotten it 50% done, as I can now detect when we're switching over hash codes. Note that it does throw an exception, that is intentional, as it is not implemented. image

Real-Packet commented 1 day ago

image now it just puts a warning about failing to transform the hash code into a string, still not implemented.

Real-Packet commented 1 day ago

It seems like when debugging the core, caseInfo is always a SwitchRegion.

Real-Packet commented 1 day ago

I "got" it working... image Probably not working because I'm currently returning a String instead of any other type that is recognized by addCaseKey

Real-Packet commented 1 day ago

It "Works"!!!! image

Real-Packet commented 1 day ago

(the reason why I put "works" in air quotes because it wasn't transforming it into a literal, but that's because I didn't use the TypeGen.literalToString function...)

Real-Packet commented 1 day ago

It works now (except it literally adds quotes instead of using TypeGen.literalToString)! image

Real-Packet commented 1 day ago

The PR is up! I would say that it'd be nice if the codebase would have a detailed CONTRIBUTING.md file, as currently it just tells you to open an issue and then maybe open a PR, things like the structure should be detailed.