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] Incorrect variable name in Java code (when compared to Simple code) causes an unintended name collision with an existing variable #1592

Closed
d3ce1t opened this issue Jul 29, 2022 · 3 comments
Labels
bug Core Issues in jadx-core module

Comments

@d3ce1t
Copy link

d3ce1t commented Jul 29, 2022

As you can see in the [Example] below there are some errors in Code view in regard to Simple view:

  • Line 2: the parcel = Parcel.obtain(); should be Parcel obtain = Parcel.obtain();.
  • Line 4: this.binder.transact(code, parcel, parcel, 0); should be this.binder.transact(code, parcel, obtain, 0);
  • Line 5: parcel.readException(); should be obtain.readException();
  • Line 6: return parcel; should be return obtain;

Because of this, the resulting Java code doesn't work as intended.

I'm using Jadx 1.4.3 as a Flatpak.

Steps to reproduce:

  • Open app-debug.zip.
  • Search for com.google.android.gms.internal.measurement.zzbm.
  • Compare code of zzb and zzc methods with their simple counterparts.

Example

Code view:

/* JADX INFO: Access modifiers changed from: protected */
public final Parcel zzb(int i, Parcel parcel) throws RemoteException {
     parcel = Parcel.obtain();
     try {
         this.zza.transact(i, parcel, parcel, 0);
         parcel.readException();
         return parcel;
     } catch (RuntimeException e) {
         throw e;
     } finally {
         parcel.recycle();
     }
}

Simple view:

public final Parcel zzb(int i, Parcel parcel) throws RemoteException {
        Parcel obtain = Parcel.obtain();
        this.zza.transact(i, parcel, obtain, 0);     // Catch: Throwable -> L8 RuntimeException -> L11
        obtain.readException();     // Catch: Throwable -> L8 RuntimeException -> L11
        parcel.recycle();
        return obtain;
    L7:
    L8:
        th = move-exception;
        parcel.recycle();
        throw th;
    L11:
        e = move-exception;
        obtain.recycle();     // Catch: Throwable -> L8
        throw e;     // Catch: Throwable -> L8
    }

Smali view:

.method protected final zzb(ILandroid/os/Parcel;)Landroid/os/Parcel;
    .registers 6
    
    .param p1, "":I
    .param p2, "":Landroid/os/Parcel;
    
                               .line 1
    002f1d04: 7100 5b07 0000          0000: invoke-static       {}, Landroid/os/Parcel;->obtain()Landroid/os/Parcel; # method@075b
    002f1d0a: 0c00                    0003: move-result-object  v0
                             try_0004: # :try_end_000c
                                       # :catch_0013
                                       # :catch_all_0011
    002f1d0c: 5431 e94c               0004: iget-object         v1, p0, Lcom/google/android/gms/internal/measurement/zzbm;->zza:Landroid/os/IBinder; # field@4ce9
    002f1d10: 1202                    0006: const/4             v2, 0
                               .line 2
    002f1d12: 7252 3007 4105          0007: invoke-interface    {v1, p1, p2, v0, v2}, Landroid/os/IBinder;->transact(I, Landroid/os/Parcel;, Landroid/os/Parcel;, I)Z # method@0730
                               .line 3
    002f1d18: 6e10 6307 0000          000a: invoke-virtual      {v0}, Landroid/os/Parcel;->readException()V # method@0763
                               .line 6
    002f1d1e: 6e10 7307 0500          000d: invoke-virtual      {p2}, Landroid/os/Parcel;->recycle()V # method@0773
    002f1d24: 1100                    0010: return-object       v0
                       catch_all_0011: # :try_0004
                                       # :try_0014
    002f1d26: 0d04                    0011: move-exception      p1
    002f1d28: 2806                    0012: goto                :goto_0018
                           catch_0013: # Ljava/lang/RuntimeException;
                                       # :try_0004
    002f1d2a: 0d04                    0013: move-exception      p1
                             try_0014: # :try_end_0017
                                       # :catch_all_0011
                               .line 4
    002f1d2c: 6e10 7307 0000          0014: invoke-virtual      {v0}, Landroid/os/Parcel;->recycle()V # method@0773
                         try_end_0017: # :try_0014
                               .line 5
    002f1d32: 2704                    0017: throw               p1
                               .line 6
                            goto_0018:
    002f1d34: 6e10 7307 0500          0018: invoke-virtual      {p2}, Landroid/os/Parcel;->recycle()V # method@0773
                               .line 7
    002f1d3a: 2704                    001b: throw               p1
    
.end method
@d3ce1t d3ce1t added bug Core Issues in jadx-core module labels Jul 29, 2022
@d3ce1t
Copy link
Author

d3ce1t commented Jul 29, 2022

For comparison, it works for a similar function:

/* JADX INFO: Access modifiers changed from: protected */
    public final void zzc(int i, Parcel parcel) throws RemoteException {
        Parcel obtain = Parcel.obtain();
        try {
            this.zza.transact(i, parcel, obtain, 0);
            obtain.readException();
        } finally {
            parcel.recycle();
            obtain.recycle();
        }
    }
public final void zzc(int i, Parcel parcel) throws RemoteException {
        Parcel obtain = Parcel.obtain();
        this.zza.transact(i, parcel, obtain, 0);     // Catch: Throwable -> L8
        obtain.readException();     // Catch: Throwable -> L8
        parcel.recycle();
        obtain.recycle();
        return;
    L8:
        th = move-exception;
        parcel.recycle();
        obtain.recycle();
        throw th;
    }
.method protected final zzc(ILandroid/os/Parcel;)V
    .registers 6
    
    .param p1, "":I
    .param p2, "":Landroid/os/Parcel;
    
                               .line 1
    002f1d84: 7100 5b07 0000          0000: invoke-static       {}, Landroid/os/Parcel;->obtain()Landroid/os/Parcel; # method@075b
    002f1d8a: 0c00                    0003: move-result-object  v0
                             try_0004: # :try_end_000c
                                       # :catch_all_0014
    002f1d8c: 5431 e94c               0004: iget-object         v1, p0, Lcom/google/android/gms/internal/measurement/zzbm;->zza:Landroid/os/IBinder; # field@4ce9
    002f1d90: 1202                    0006: const/4             v2, 0
                               .line 2
    002f1d92: 7252 3007 4105          0007: invoke-interface    {v1, p1, p2, v0, v2}, Landroid/os/IBinder;->transact(I, Landroid/os/Parcel;, Landroid/os/Parcel;, I)Z # method@0730
                               .line 3
    002f1d98: 6e10 6307 0000          000a: invoke-virtual      {v0}, Landroid/os/Parcel;->readException()V # method@0763
                               .line 4
    002f1d9e: 6e10 7307 0500          000d: invoke-virtual      {p2}, Landroid/os/Parcel;->recycle()V # method@0773
                               .line 5
    002f1da4: 6e10 7307 0000          0010: invoke-virtual      {v0}, Landroid/os/Parcel;->recycle()V # method@0773
    002f1daa: 0e00                    0013: return-void         
                       catch_all_0014: # :try_0004
    002f1dac: 0d04                    0014: move-exception      p1
                               .line 4
    002f1dae: 6e10 7307 0500          0015: invoke-virtual      {p2}, Landroid/os/Parcel;->recycle()V # method@0773
                               .line 5
    002f1db4: 6e10 7307 0000          0018: invoke-virtual      {v0}, Landroid/os/Parcel;->recycle()V # method@0773
                               .line 6
    002f1dba: 2704                    001b: throw               p1
    
.end method

@d3ce1t d3ce1t changed the title [core] Variable name in Code view doesn't match the one in Simple view [core] Incorrect variable name in Java code (when compared to Simple code) causes an unintended name collision with existing variable Jul 29, 2022
@d3ce1t d3ce1t changed the title [core] Incorrect variable name in Java code (when compared to Simple code) causes an unintended name collision with existing variable [core] Incorrect variable name in Java code (when compared to Simple code) causes an unintended name collision with an existing variable Jul 29, 2022
@skylot
Copy link
Owner

skylot commented Jul 30, 2022

@d3ce1t thanks for report, this is very nice test case!

I commit a fix, not sure if it will work for all cases, but at least this one is fixed.
Also, I add option to cli and gui to disable finally block extraction, this can be used as workaround for similar cases or any other issues with finally restore.

P.S. I am glad to see that someone is using simple mode, because lack of feedback make me thought that this mode was not very usable 🙂

@skylot skylot closed this as completed Jul 30, 2022
@d3ce1t
Copy link
Author

d3ce1t commented Aug 1, 2022

That was fast!! Thanks A LOT for the quick fix @skylot. It works :)

I'm developing a gradle plugin to intercept http requests in an android app. I'm using bytecode transformation with ASM and couldn't do that without jadx. It helps me to see what third-party libraries are used and where to apply my transformations. Although I spend most of the time searching and switching between code and smali views, the simple mode was helpful in spotting this bug :).

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
Projects
None yet
Development

No branches or pull requests

2 participants