From 52ba33c5a3ccf55f5cc1fdaf6223d818f34c548a Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 3 May 2019 22:40:18 +0300 Subject: [PATCH] fix: avoid local variables collision with full class names (#647) --- .../main/java/jadx/core/codegen/NameGen.java | 6 +- .../java/jadx/core/dex/nodes/RootNode.java | 6 ++ .../jadx/core/dex/visitors/RenameVisitor.java | 19 ++--- .../java/jadx/core/utils/CacheStorage.java | 17 +++++ .../names/TestLocalVarCollideWithPackage.java | 71 +++++++++++++++++++ .../TestLocalVarCollideWithPackage/1.smali | 18 +++++ .../TestLocalVarCollideWithPackage/2.smali | 10 +++ .../TestLocalVarCollideWithPackage/3.smali | 4 ++ 8 files changed, 141 insertions(+), 10 deletions(-) create mode 100644 jadx-core/src/main/java/jadx/core/utils/CacheStorage.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/names/TestLocalVarCollideWithPackage.java create mode 100644 jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/1.smali create mode 100644 jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/2.smali create mode 100644 jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/3.smali diff --git a/jadx-core/src/main/java/jadx/core/codegen/NameGen.java b/jadx-core/src/main/java/jadx/core/codegen/NameGen.java index a2da72f7b..701142176 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/NameGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/NameGen.java @@ -1,6 +1,6 @@ package jadx.core.codegen; -import java.util.LinkedHashSet; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -30,7 +30,7 @@ public class NameGen { private static final Map OBJ_ALIAS; - private final Set varNames = new LinkedHashSet<>(); + private final Set varNames = new HashSet<>(); private final MethodNode mth; private final boolean fallback; @@ -67,6 +67,8 @@ public class NameGen { for (ClassNode innerClass : parentClass.getInnerClasses()) { varNames.add(innerClass.getAlias().getShortName()); } + // add all root package names to avoid collisions with full class names + varNames.addAll(mth.root().getCacheStorage().getRootPkgs()); } public String assignArg(CodeVar var) { diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java index 067d4f42c..1b0c8ef7c 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java @@ -19,6 +19,7 @@ import jadx.core.dex.info.FieldInfo; import jadx.core.dex.info.InfoStorage; import jadx.core.dex.info.MethodInfo; import jadx.core.dex.visitors.typeinference.TypeUpdate; +import jadx.core.utils.CacheStorage; import jadx.core.utils.ErrorsCounter; import jadx.core.utils.StringUtils; import jadx.core.utils.android.AndroidResourcesUtils; @@ -36,6 +37,7 @@ public class RootNode { private final StringUtils stringUtils; private final ConstStorage constValues; private final InfoStorage infoStorage = new InfoStorage(); + private final CacheStorage cacheStorage = new CacheStorage(); private final TypeUpdate typeUpdate; private ClspGraph clsp; @@ -222,6 +224,10 @@ public class RootNode { return infoStorage; } + public CacheStorage getCacheStorage() { + return cacheStorage; + } + public JadxArgs getArgs() { return args; } 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 a713f9d68..53f509845 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 @@ -67,9 +67,7 @@ public class RenameVisitor extends AbstractVisitor { } } } - if (args.isRenameValid()) { - checkFieldsCollisionWithRootPackage(classes); - } + processRootPackages(root, classes); } private void checkClassName(ClassNode cls, JadxArgs args) { @@ -140,12 +138,17 @@ public class RenameVisitor extends AbstractVisitor { } } - private void checkFieldsCollisionWithRootPackage(List classes) { + private void processRootPackages(RootNode root, List classes) { Set rootPkgs = collectRootPkgs(classes); - for (ClassNode cls : classes) { - for (FieldNode field : cls.getFields()) { - if (rootPkgs.contains(field.getAlias())) { - deobfuscator.forceRenameField(field); + root.getCacheStorage().setRootPkgs(rootPkgs); + + if (root.getArgs().isRenameValid()) { + // rename field if collide with any root package + for (ClassNode cls : classes) { + for (FieldNode field : cls.getFields()) { + if (rootPkgs.contains(field.getAlias())) { + deobfuscator.forceRenameField(field); + } } } } diff --git a/jadx-core/src/main/java/jadx/core/utils/CacheStorage.java b/jadx-core/src/main/java/jadx/core/utils/CacheStorage.java new file mode 100644 index 000000000..09070376f --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/utils/CacheStorage.java @@ -0,0 +1,17 @@ +package jadx.core.utils; + +import java.util.Collections; +import java.util.Set; + +public class CacheStorage { + + private Set rootPkgs = Collections.emptySet(); + + public Set getRootPkgs() { + return rootPkgs; + } + + public void setRootPkgs(Set rootPkgs) { + this.rootPkgs = rootPkgs; + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/TestLocalVarCollideWithPackage.java b/jadx-core/src/test/java/jadx/tests/integration/names/TestLocalVarCollideWithPackage.java new file mode 100644 index 000000000..8528e62d6 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/names/TestLocalVarCollideWithPackage.java @@ -0,0 +1,71 @@ +package jadx.tests.integration.names; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + +public class TestLocalVarCollideWithPackage extends SmaliTest { + //@formatter:off + /* + ----------------------------------------------------------- + package first; + + import pkg.Second; + + public class A { + public String test() { + Second second = new Second(); + second.A.call(); // collision + return second.str; + } + } + ----------------------------------------------------------- + package pkg; + + public class Second { + public String str; + } + ----------------------------------------------------------- + package second; + + public class A { + } + ----------------------------------------------------------- + */ + //@formatter:on + + @Test + public void test() { + List clsList = loadFromSmaliFiles(); + ClassNode firstA = searchCls(clsList, "first.A"); + String code = firstA.getCode().toString(); + + assertThat(code, containsString("second.A.call();")); + assertThat(code, not(containsString("Second second = new Second();"))); + } + + @Test + public void testNoDebug() { + noDebugInfo(); + loadFromSmaliFiles(); + } + + @Test + public void testWithoutImports() { + getArgs().setUseImports(false); + loadFromSmaliFiles(); + } + + @Test + public void testWithDeobfuscation() { + enableDeobfuscation(); + loadFromSmaliFiles(); + } +} diff --git a/jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/1.smali b/jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/1.smali new file mode 100644 index 000000000..8554063a5 --- /dev/null +++ b/jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/1.smali @@ -0,0 +1,18 @@ +.class public Lfirst/A; +.super Ljava/lang/Object; + +.method public test()Ljava/lang/String; + .registers 2 + + new-instance v1, Lpkg/Second; + + invoke-direct {v1}, Lpkg/Second;->()V + + .local v1, "second":Lpkg/Second; + + invoke-static {}, Lsecond/A;->call()Ljava/lang/String; + + iget-object v0, v1, Lpkg/Second;->str:Ljava/lang/String; + + return-object v0 +.end method diff --git a/jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/2.smali b/jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/2.smali new file mode 100644 index 000000000..59046fb39 --- /dev/null +++ b/jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/2.smali @@ -0,0 +1,10 @@ +.class public Lsecond/A; +.super Ljava/lang/Object; + +.method static public call()Ljava/lang/String; + .registers 1 + + const v0, 0 + + return-object v0 +.end method diff --git a/jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/3.smali b/jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/3.smali new file mode 100644 index 000000000..5b8e88684 --- /dev/null +++ b/jadx-core/src/test/smali/names/TestLocalVarCollideWithPackage/3.smali @@ -0,0 +1,4 @@ +.class public Lpkg/Second; +.super Ljava/lang/Object; + +.field public str:Ljava/lang/String;