skylot / jadx

Dex to Java decompiler
Apache License 2.0
41.99k stars 4.9k forks source link

[core] New Proguard technique causing type confusion and failures to decompile #2285

Closed bluemods closed 2 months ago

bluemods commented 2 months ago

Issue details

Seems as though ProGuard has implemented a new technique by changing the constructor instructions for zero-args constructors.

See the commented sample and the screenshot of the decompiler being confused by it and failing type inference.

Normal (expected opcodes for instantiating an object with a zero-args constructor):

new-instance v0, Lkik/android/util/PartitionedRecycledViewPool$a;
invoke-direct {v0}, Lkik/android/util/PartitionedRecycledViewPool$a;-><init>()V

Proguarded:

new-instance v0, Lkik/android/util/PartitionedRecycledViewPool$a;
invoke-direct {v0}, Ljava/lang/Object;-><init>()V

ss

example.smali

Relevant log output or stacktrace

No response

Provide sample and class/method full name

.method public final a(III)V
    .registers 6

    new-instance v0, Lkik/android/util/PartitionedRecycledViewPool$a;

    # This should normally read as follows:
    # invoke-direct {v0}, Lkik/android/util/PartitionedRecycledViewPool$a;-><init>()V
    # 
    # but, ProGuard is initializing with java/lang/Object
    #
    # The decompiler then assumes that it is an object being instantiated and loses the actual object type, which is kik.android.util.PartitionedRecycledViewPool.a

    invoke-direct {v0}, Ljava/lang/Object;-><init>()V

    if-gt p2, p3, :cond_1c

    new-instance p2, Ljava/util/ArrayDeque;

    iget v1, v0, Lkik/android/util/PartitionedRecycledViewPool$a;->a:I

    invoke-direct {p2, v1}, Ljava/util/ArrayDeque;-><init>(I)V

    iput-object p2, v0, Lkik/android/util/PartitionedRecycledViewPool$a;->b:Ljava/util/ArrayDeque;

    iput p3, v0, Lkik/android/util/PartitionedRecycledViewPool$a;->a:I

    iget-object p2, p0, Lkik/android/util/PartitionedRecycledViewPool;->a:Ljava/util/HashMap;

    invoke-static {p1}, Ljava/lang/Integer;->valueOf(I)Ljava/lang/Integer;

    move-result-object p1

    invoke-virtual {p2, p1, v0}, Ljava/util/HashMap;->put(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;

    .line 28
    return-void

    :cond_1c
    new-instance p1, Ljava/lang/IllegalArgumentException;

    const-string/jumbo p2, "preload size exceeds cache size"

    invoke-direct {p1, p2}, Ljava/lang/IllegalArgumentException;-><init>(Ljava/lang/String;)V

    throw p1
.end method

Jadx version

1.5.0

iznaka commented 2 months ago

What makes this potentially acceptable is the fact that the fields of PartitionedRecycledViewPool$a are not initialized to specific values.

So, calling Object.<init> on the allocated memory instance would be sufficient here. I've never seen this in the wild. Could you share a full APK? Are you sure ProGuard is doing this?

I've just verified it with JEB Pro, the result is quite explicit, the alloc and init parts are broken down to warn the user that something is off: image

skylot commented 2 months ago

Fixed. @bluemods thank you for reporting this issue :+1: