From da41efa3db7f8f2c978667b5c3a74c3398e9c068 Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 18 Jan 2019 14:04:00 +0300 Subject: [PATCH] fix: force rename by checks from RenameVisitor (#432) --- .../java/jadx/core/deobf/Deobfuscator.java | 11 ++++ .../java/jadx/core/dex/info/FieldInfo.java | 4 ++ .../java/jadx/core/dex/nodes/ClassNode.java | 9 ++++ .../java/jadx/core/dex/nodes/DexNode.java | 7 ++- .../jadx/core/dex/visitors/RenameVisitor.java | 24 ++++----- .../names/TestDuplicatedNames.java | 51 +++++++++++++++++++ .../smali/names/TestDuplicatedNames.smali | 40 +++++++++++++++ 7 files changed, 128 insertions(+), 18 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/names/TestDuplicatedNames.java create mode 100644 jadx-core/src/test/smali/names/TestDuplicatedNames.smali diff --git a/jadx-core/src/main/java/jadx/core/deobf/Deobfuscator.java b/jadx-core/src/main/java/jadx/core/deobf/Deobfuscator.java index 7e18b1930..859341f95 100644 --- a/jadx-core/src/main/java/jadx/core/deobf/Deobfuscator.java +++ b/jadx-core/src/main/java/jadx/core/deobf/Deobfuscator.java @@ -243,6 +243,10 @@ public class Deobfuscator { } } + public void forceRenameField(FieldNode field) { + field.getFieldInfo().setAlias(makeFieldAlias(field)); + } + public void renameMethod(MethodNode mth) { String alias = getMethodAlias(mth); if (alias != null) { @@ -253,6 +257,13 @@ public class Deobfuscator { } } + public void forceRenameMethod(MethodNode mth) { + mth.getMethodInfo().setAlias(makeMethodAlias(mth)); + if (mth.isVirtual()) { + resolveOverriding(mth); + } + } + public void addPackagePreset(String origPkgName, String pkgAlias) { PackageNode pkg = getPackageNode(origPkgName, true); pkg.setAlias(pkgAlias); diff --git a/jadx-core/src/main/java/jadx/core/dex/info/FieldInfo.java b/jadx-core/src/main/java/jadx/core/dex/info/FieldInfo.java index fd35db1de..451c757b2 100644 --- a/jadx-core/src/main/java/jadx/core/dex/info/FieldInfo.java +++ b/jadx-core/src/main/java/jadx/core/dex/info/FieldInfo.java @@ -65,6 +65,10 @@ public final class FieldInfo { return !name.equals(alias); } + public boolean equalsNameAndType(FieldInfo other) { + return name.equals(other.name) && type.equals(other.type); + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java index 51a179084..dc5e0a70a 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java @@ -321,6 +321,15 @@ public class ClassNode extends LineAttrNode implements ILoadable, IDexNode { return null; } + public FieldNode searchFieldByNameAndType(FieldInfo field) { + for (FieldNode f : fields) { + if (f.getFieldInfo().equalsNameAndType(field)) { + return f; + } + } + return null; + } + public FieldNode searchFieldByName(String name) { for (FieldNode f : fields) { if (f.getName().equals(name)) { diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/DexNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/DexNode.java index ce0d5ea34..832ec122d 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/DexNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/DexNode.java @@ -150,16 +150,15 @@ public class DexNode implements IDexNode { @Nullable FieldNode deepResolveField(@NotNull ClassNode cls, FieldInfo fieldInfo) { - FieldNode field = cls.searchFieldByName(fieldInfo.getName()); + FieldNode field = cls.searchFieldByNameAndType(fieldInfo); if (field != null) { return field; } - FieldNode found; ArgType superClass = cls.getSuperClass(); if (superClass != null) { ClassNode superNode = resolveClass(superClass); if (superNode != null) { - found = deepResolveField(superNode, fieldInfo); + FieldNode found = deepResolveField(superNode, fieldInfo); if (found != null) { return found; } @@ -168,7 +167,7 @@ public class DexNode implements IDexNode { for (ArgType iFaceType : cls.getInterfaces()) { ClassNode iFaceNode = resolveClass(iFaceType); if (iFaceNode != null) { - found = deepResolveField(iFaceNode, fieldInfo); + FieldNode found = deepResolveField(iFaceNode, fieldInfo); if (found != null) { return found; } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/RenameVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/RenameVisitor.java index a3ca4fa91..802e27942 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/RenameVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/RenameVisitor.java @@ -12,6 +12,7 @@ import jadx.core.Consts; import jadx.core.deobf.Deobfuscator; import jadx.core.deobf.NameMapper; import jadx.core.dex.attributes.AFlag; +import jadx.core.dex.info.AccessInfo; import jadx.core.dex.info.ClassInfo; import jadx.core.dex.info.FieldInfo; import jadx.core.dex.nodes.ClassNode; @@ -19,7 +20,6 @@ import jadx.core.dex.nodes.DexNode; import jadx.core.dex.nodes.FieldNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.nodes.RootNode; -import jadx.core.utils.exceptions.JadxException; import jadx.core.utils.files.FileUtils; import jadx.core.utils.files.InputFile; @@ -49,20 +49,12 @@ public class RenameVisitor extends AbstractVisitor { checkClasses(root, isCaseSensitive); } - @Override - public boolean visit(ClassNode cls) throws JadxException { - checkFields(cls); - checkMethods(cls); - for (ClassNode inner : cls.getInnerClasses()) { - visit(inner); - } - return false; - } - private void checkClasses(RootNode root, boolean caseSensitive) { Set clsNames = new HashSet<>(); for (ClassNode cls : root.getClasses(true)) { checkClassName(cls); + checkFields(cls); + checkMethods(cls); if (!caseSensitive) { ClassInfo classInfo = cls.getClassInfo(); String clsFileName = classInfo.getAlias().getFullPath(); @@ -103,7 +95,7 @@ public class RenameVisitor extends AbstractVisitor { FieldInfo fieldInfo = field.getFieldInfo(); String fieldName = fieldInfo.getAlias(); if (!names.add(fieldName) || !NameMapper.isValidIdentifier(fieldName)) { - deobfuscator.renameField(field); + deobfuscator.forceRenameField(field); } } } @@ -111,12 +103,16 @@ public class RenameVisitor extends AbstractVisitor { private void checkMethods(ClassNode cls) { Set names = new HashSet<>(); for (MethodNode mth : cls.getMethods()) { - if (mth.contains(AFlag.DONT_GENERATE) || mth.getAccessFlags().isConstructor()) { + AccessInfo accessFlags = mth.getAccessFlags(); + if (accessFlags.isConstructor() + || accessFlags.isBridge() + || accessFlags.isSynthetic() + || mth.contains(AFlag.DONT_GENERATE)) { continue; } String signature = mth.getMethodInfo().makeSignature(false); if (!names.add(signature) || !NameMapper.isValidIdentifier(mth.getAlias())) { - deobfuscator.renameMethod(mth); + deobfuscator.forceRenameMethod(mth); } } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/TestDuplicatedNames.java b/jadx-core/src/test/java/jadx/tests/integration/names/TestDuplicatedNames.java new file mode 100644 index 000000000..7adc7f184 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/names/TestDuplicatedNames.java @@ -0,0 +1,51 @@ +package jadx.tests.integration.names; + +import org.junit.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.junit.Assert.assertThat; + +public class TestDuplicatedNames extends SmaliTest { +/* + public static class TestCls { + + public Object fieldName; + public String fieldName; + + public Object run() { + return this.fieldName; + } + + public String run() { + return this.fieldName; + } + } +*/ + @Test + public void test() { + commonChecks(); + } + + @Test + public void testWithDeobf() { + enableDeobfuscation(); + commonChecks(); + } + + private void commonChecks() { + ClassNode cls = getClassNodeFromSmaliWithPath("names", "TestDuplicatedNames"); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("Object fieldName;")); + assertThat(code, containsOne("String f0fieldName")); + + assertThat(code, containsOne("this.fieldName")); + assertThat(code, containsOne("this.f0fieldName")); + + assertThat(code, containsOne("public Object run() {")); + assertThat(code, containsOne("public String m0run() {")); + } +} diff --git a/jadx-core/src/test/smali/names/TestDuplicatedNames.smali b/jadx-core/src/test/smali/names/TestDuplicatedNames.smali new file mode 100644 index 000000000..065a3445d --- /dev/null +++ b/jadx-core/src/test/smali/names/TestDuplicatedNames.smali @@ -0,0 +1,40 @@ +.class public LTestDuplicatedNames; +.super Ljava/lang/Object; +.source "TestDuplicatedNames.java" + + +# instance fields +.field public fieldName:Ljava/lang/String; +.field public fieldName:Ljava/lang/Object; + + +# direct methods +.method public constructor ()V + .registers 1 + + .prologue + .line 3 + invoke-direct {p0}, Ljava/lang/Object;->()V + + return-void +.end method + + +# virtual methods +.method public run()Ljava/lang/String; + .registers 2 + + .prologue + iget-object v0, p0, LTestDuplicatedNames;->fieldName:Ljava/lang/String; + + return-object v0 +.end method + +.method public run()Ljava/lang/Object; + .registers 2 + + .prologue + iget-object v0, p0, LTestDuplicatedNames;->fieldName:Ljava/lang/Object; + + return-object v0 +.end method