Commit 52ba33c5 authored by Skylot's avatar Skylot

fix: avoid local variables collision with full class names (#647)

parent 156c9798
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<String, String> OBJ_ALIAS;
private final Set<String> varNames = new LinkedHashSet<>();
private final Set<String> 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) {
......
......@@ -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;
}
......
......@@ -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<ClassNode> classes) {
private void processRootPackages(RootNode root, List<ClassNode> classes) {
Set<String> 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);
}
}
}
}
......
package jadx.core.utils;
import java.util.Collections;
import java.util.Set;
public class CacheStorage {
private Set<String> rootPkgs = Collections.emptySet();
public Set<String> getRootPkgs() {
return rootPkgs;
}
public void setRootPkgs(Set<String> rootPkgs) {
this.rootPkgs = rootPkgs;
}
}
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<ClassNode> 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();
}
}
.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;-><init>()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
.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
.class public Lpkg/Second;
.super Ljava/lang/Object;
.field public str:Ljava/lang/String;
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment