jessikitty/jadx
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix: don't trust type info in signature, check before apply (#858)

Browse Source
This commit is contained in:
Skylot
2020-08-17 21:11:33 +01:00
parent 94a06d9b6f
commit 2d641bf049
11 changed files with 266 additions and 130 deletions
Download Patch File Download Diff File Expand all files Collapse all files
jadx-core/src/main/java/jadx/api/JadxDecompiler.java
+1 -1
View File
@@ -100,8 +100,8 @@ public final class JadxDecompiler implements Closeable {
root.loadClasses(loadedInputs);
root.initClassPath();
root.loadResources(getResources());
root.initPasses();
root.runPreDecompileStage();
root.initPasses();
}
private void loadInputFiles() {
jadx-core/src/main/java/jadx/core/Jadx.java
+2
View File
@@ -37,6 +37,7 @@ import jadx.core.dex.visitors.ProcessInstructionsVisitor;
import jadx.core.dex.visitors.ReSugarCode;
import jadx.core.dex.visitors.RenameVisitor;
import jadx.core.dex.visitors.ShadowFieldVisitor;
import jadx.core.dex.visitors.SignatureProcessor;
import jadx.core.dex.visitors.SimplifyVisitor;
import jadx.core.dex.visitors.blocksmaker.BlockExceptionHandler;
import jadx.core.dex.visitors.blocksmaker.BlockFinish;
@@ -78,6 +79,7 @@ public class Jadx {
public static List<IDexTreeVisitor> getPreDecompilePassesList() {
List<IDexTreeVisitor> passes = new ArrayList<>();
passes.add(new SignatureProcessor());
passes.add(new RenameVisitor());
passes.add(new UsageInfoVisitor());
return passes;
jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java
+6 -61
View File
@@ -6,7 +6,6 @@ import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Consumer;
import java.util.stream.Collectors;
@@ -35,7 +34,6 @@ import jadx.core.dex.info.FieldInfo;
import jadx.core.dex.info.MethodInfo;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.LiteralArg;
import jadx.core.dex.nodes.parser.SignatureParser;
import jadx.core.dex.visitors.ProcessAnonymous;
import jadx.core.utils.Utils;
import jadx.core.utils.exceptions.JadxRuntimeException;
@@ -108,9 +106,6 @@ public class ClassNode extends NotificationAttrNode implements ILoadable, ICodeN
AnnotationsList.attach(this, cls.getAnnotations());
loadStaticValues(cls, fields);
initAccessFlags(cls);
parseClassSignature();
setFieldsTypesFromSignature();
methods.forEach(MethodNode::initMethodTypes);
addSourceFilenameAttr(cls.getSourceFile());
buildCache();
@@ -119,6 +114,12 @@ public class ClassNode extends NotificationAttrNode implements ILoadable, ICodeN
}
}
public void updateGenericClsData(ArgType superClass, List<ArgType> interfaces, List<ArgType> generics) {
this.superClass = superClass;
this.interfaces = interfaces;
this.generics = generics;
}
/**
* Restore original access flags from Dalvik annotation if present
*/
@@ -176,62 +177,6 @@ public class ClassNode extends NotificationAttrNode implements ILoadable, ICodeN
root().getConstValues().processConstFields(this, staticFields);
}
/**
* Class signature format:
* https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.9.1
*/
private void parseClassSignature() {
SignatureParser sp = SignatureParser.fromNode(this);
if (sp == null) {
return;
}
try {
// parse class generic map
generics = sp.consumeGenericTypeParameters();
// parse super class signature
superClass = validateSuperCls(sp.consumeType(), superClass);
// parse interfaces signatures
for (int i = 0; i < interfaces.size(); i++) {
ArgType type = sp.consumeType();
if (type != null) {
interfaces.set(i, type);
} else {
break;
}
}
} catch (Exception e) {
LOG.error("Class signature parse error: {}", this, e);
}
}
private ArgType validateSuperCls(ArgType candidateType, ArgType currentType) {
if (!candidateType.isObject()) {
this.addComment("Incorrect class signature, super class is not object: " + SignatureParser.getSignature(this));
return currentType;
}
if (Objects.equals(candidateType.getObject(), this.getClassInfo().getType().getObject())) {
this.addComment("Incorrect class signature, super class is equals to this class: " + SignatureParser.getSignature(this));
return currentType;
}
return candidateType;
}
private void setFieldsTypesFromSignature() {
for (FieldNode field : fields) {
try {
SignatureParser sp = SignatureParser.fromNode(field);
if (sp != null) {
ArgType gType = sp.consumeType();
if (gType != null) {
field.setType(root.getTypeUtils().expandTypeVariables(this, gType));
}
}
} catch (Exception e) {
LOG.error("Field signature parse error: {}.{}", this.getFullName(), field.getName(), e);
}
}
}
private void addSourceFilenameAttr(String fileName) {
if (fileName == null) {
return;
jadx-core/src/main/java/jadx/core/dex/nodes/FieldNode.java
+4 -4
View File
@@ -35,6 +35,10 @@ public class FieldNode extends LineAttrNode implements ICodeNode {
this.accFlags = new AccessInfo(accessFlags, AFType.FIELD);
}
public void updateType(ArgType type) {
this.type = type;
}
public FieldInfo getFieldInfo() {
return fieldInfo;
}
@@ -65,10 +69,6 @@ public class FieldNode extends LineAttrNode implements ICodeNode {
return type;
}
public void setType(ArgType type) {
this.type = type;
}
public ClassNode getParentClass() {
return parentClass;
}
jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java
+13 -59
View File
@@ -30,7 +30,6 @@ import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.InsnArg;
import jadx.core.dex.instructions.args.RegisterArg;
import jadx.core.dex.instructions.args.SSAVar;
import jadx.core.dex.nodes.parser.SignatureParser;
import jadx.core.dex.nodes.utils.TypeUtils;
import jadx.core.dex.regions.Region;
import jadx.core.dex.trycatch.ExceptionHandler;
@@ -96,6 +95,10 @@ public class MethodNode extends NotificationAttrNode implements IMethodDetails,
this.codeReader = codeReader.copy();
this.insnsCount = codeReader.getInsnsCount();
}
this.retType = mthInfo.getReturnType();
this.argTypes = mthInfo.getArgumentsTypes();
this.typeParameters = Collections.emptyList();
unload();
}
@@ -119,6 +122,15 @@ public class MethodNode extends NotificationAttrNode implements IMethodDetails,
unloadAttributes();
}
public void updateTypes(List<ArgType> argTypes, ArgType retType) {
this.argTypes = argTypes;
this.retType = retType;
}
public void updateTypeParameters(List<ArgType> typeParameters) {
this.typeParameters = typeParameters;
}
@Override
public void load() throws DecodeException {
if (loaded) {
@@ -180,64 +192,6 @@ public class MethodNode extends NotificationAttrNode implements IMethodDetails,
}
}
public void initMethodTypes() {
if (!parseSignature()) {
this.retType = mthInfo.getReturnType();
this.argTypes = mthInfo.getArgumentsTypes();
this.typeParameters = Collections.emptyList();
}
}
private boolean parseSignature() {
SignatureParser sp = SignatureParser.fromNode(this);
if (sp == null) {
return false;
}
try {
this.typeParameters = sp.consumeGenericTypeParameters();
List<ArgType> parsedArgTypes = sp.consumeMethodArgs();
ArgType parsedRetType = sp.consumeType();
List<ArgType> mthArgs = mthInfo.getArgumentsTypes();
if (parsedArgTypes.size() != mthArgs.size()) {
if (parsedArgTypes.isEmpty()) {
return false;
}
if (!tryFixArgsCounts(parsedArgTypes, mthArgs)) {
addComment("Incorrect method signature, types: " + Utils.listToString(parsedArgTypes));
return false;
}
}
TypeUtils typeUtils = root().getTypeUtils();
this.retType = typeUtils.expandTypeVariables(this, parsedRetType);
this.argTypes = Collections.unmodifiableList(Utils.collectionMap(parsedArgTypes,
t -> typeUtils.expandTypeVariables(this, t)));
return true;
} catch (Exception e) {
addWarnComment("Failed to parse method signature: " + sp.getSignature(), e);
return false;
}
}
private boolean tryFixArgsCounts(List<ArgType> argsTypes, List<ArgType> mthArgs) {
if (!mthInfo.isConstructor()) {
return false;
}
if (getParentClass().getAccessFlags().isEnum()) {
if (mthArgs.size() >= 2) {
// TODO:
argsTypes.add(0, mthArgs.get(0));
argsTypes.add(1, mthArgs.get(1));
}
} else {
if (!mthArgs.isEmpty()) {
// add synthetic arg for outer class
argsTypes.add(0, mthArgs.get(0));
}
}
return argsTypes.size() == mthArgs.size();
}
private void initArguments(List<ArgType> args) {
int pos;
if (noCode) {
jadx-core/src/main/java/jadx/core/dex/nodes/utils/TypeUtils.java
+5 -5
View File
@@ -13,6 +13,7 @@ import org.jetbrains.annotations.Nullable;
import jadx.core.clsp.ClspClass;
import jadx.core.dex.attributes.AType;
import jadx.core.dex.attributes.nodes.MethodTypeVarsAttr;
import jadx.core.dex.attributes.nodes.NotificationAttrNode;
import jadx.core.dex.instructions.BaseInvokeNode;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.InsnArg;
@@ -21,7 +22,6 @@ import jadx.core.dex.nodes.IMethodDetails;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.nodes.RootNode;
import jadx.core.utils.Utils;
import jadx.core.utils.exceptions.JadxRuntimeException;
import static jadx.core.utils.Utils.isEmpty;
import static jadx.core.utils.Utils.notEmpty;
@@ -48,19 +48,19 @@ public class TypeUtils {
public ArgType expandTypeVariables(ClassNode cls, ArgType type) {
if (type.containsTypeVariable()) {
expandTypeVar(type, cls.getGenericTypeParameters());
expandTypeVar(cls, type, cls.getGenericTypeParameters());
}
return type;
}
public ArgType expandTypeVariables(MethodNode mth, ArgType type) {
if (type.containsTypeVariable()) {
expandTypeVar(type, getKnownTypeVarsAtMethod(mth));
expandTypeVar(mth, type, getKnownTypeVarsAtMethod(mth));
}
return type;
}
private void expandTypeVar(ArgType type, Collection<ArgType> typeVars) {
private void expandTypeVar(NotificationAttrNode node, ArgType type, Collection<ArgType> typeVars) {
boolean allExtendsEmpty = true;
for (ArgType argType : typeVars) {
if (notEmpty(argType.getExtendTypes())) {
@@ -80,7 +80,7 @@ public class TypeUtils {
return null;
}
}
throw new JadxRuntimeException("Can't find type var definition: " + typeVarName);
node.addWarnComment("Unknown type variable: " + typeVarName + " in type: " + type);
}
return null;
});
jadx-core/src/main/java/jadx/core/dex/visitors/SignatureProcessor.java
+163
View File
@@ -0,0 +1,163 @@
package jadx.core.dex.visitors;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import jadx.core.dex.info.MethodInfo;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.nodes.ClassNode;
import jadx.core.dex.nodes.FieldNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.nodes.RootNode;
import jadx.core.dex.nodes.parser.SignatureParser;
import jadx.core.dex.nodes.utils.TypeUtils;
import jadx.core.dex.visitors.typeinference.TypeCompareEnum;
import jadx.core.utils.Utils;
import static java.util.Collections.unmodifiableList;
public class SignatureProcessor extends AbstractVisitor {
private RootNode root;
@Override
public void init(RootNode root) {
this.root = root;
for (ClassNode cls : this.root.getClasses()) {
processCls(cls);
}
}
private void processCls(ClassNode cls) {
parseClassSignature(cls);
for (FieldNode field : cls.getFields()) {
parseFieldSignature(field);
}
for (MethodNode mth : cls.getMethods()) {
parseMethodSignature(mth);
}
}
private void parseClassSignature(ClassNode cls) {
SignatureParser sp = SignatureParser.fromNode(cls);
if (sp == null) {
return;
}
try {
List<ArgType> generics = sp.consumeGenericTypeParameters();
ArgType superClass = validateClsType(cls, sp.consumeType(), cls.getSuperClass());
List<ArgType> interfaces = cls.getInterfaces();
for (int i = 0; i < interfaces.size(); i++) {
ArgType type = sp.consumeType();
if (type != null) {
interfaces.set(i, validateClsType(cls, type, interfaces.get(i)));
} else {
break;
}
}
cls.updateGenericClsData(superClass, interfaces, generics);
} catch (Exception e) {
cls.addWarnComment("Failed to parse class signature: " + sp.getSignature(), e);
}
}
private ArgType validateClsType(ClassNode cls, ArgType candidateType, ArgType currentType) {
if (!candidateType.isObject()) {
cls.addWarnComment("Incorrect class signature, class is not object: " + SignatureParser.getSignature(cls));
return currentType;
}
if (Objects.equals(candidateType.getObject(), cls.getClassInfo().getType().getObject())) {
cls.addWarnComment("Incorrect class signature, class is equals to this class: " + SignatureParser.getSignature(cls));
return currentType;
}
return candidateType;
}
private void parseFieldSignature(FieldNode field) {
SignatureParser sp = SignatureParser.fromNode(field);
if (sp == null) {
return;
}
ClassNode cls = field.getParentClass();
try {
ArgType gType = sp.consumeType();
if (gType == null) {
return;
}
ArgType type = root.getTypeUtils().expandTypeVariables(cls, gType);
if (!validateParsedType(type, field.getType())) {
cls.addWarnComment("Incorrect field signature: " + sp.getSignature());
return;
}
field.updateType(type);
} catch (Exception e) {
cls.addWarnComment("Field signature parse error: " + field.getName(), e);
}
}
private void parseMethodSignature(MethodNode mth) {
SignatureParser sp = SignatureParser.fromNode(mth);
if (sp == null) {
return;
}
try {
List<ArgType> typeParameters = sp.consumeGenericTypeParameters();
List<ArgType> parsedArgTypes = sp.consumeMethodArgs();
ArgType parsedRetType = sp.consumeType();
if (!validateParsedType(parsedRetType, mth.getMethodInfo().getReturnType())) {
mth.addWarnComment("Incorrect return type in method signature: " + sp.getSignature());
return;
}
List<ArgType> checkedArgTypes = checkArgTypes(mth, sp, parsedArgTypes);
if (checkedArgTypes == null) {
return;
}
mth.updateTypeParameters(typeParameters); // apply before expand args
TypeUtils typeUtils = root.getTypeUtils();
ArgType retType = typeUtils.expandTypeVariables(mth, parsedRetType);
List<ArgType> resultArgTypes = Utils.collectionMap(checkedArgTypes, t -> typeUtils.expandTypeVariables(mth, t));
mth.updateTypes(unmodifiableList(resultArgTypes), retType);
} catch (Exception e) {
mth.addWarnComment("Failed to parse method signature: " + sp.getSignature(), e);
}
}
private List<ArgType> checkArgTypes(MethodNode mth, SignatureParser sp, List<ArgType> parsedArgTypes) {
MethodInfo mthInfo = mth.getMethodInfo();
List<ArgType> mthArgTypes = mthInfo.getArgumentsTypes();
int len = parsedArgTypes.size();
if (len != mthArgTypes.size()) {
if (mth.getParentClass().getAccessFlags().isEnum()) {
// ignore for enums
return null;
}
if (mthInfo.isConstructor() && !mthArgTypes.isEmpty() && !parsedArgTypes.isEmpty()) {
// add synthetic arg for outer class (see test TestGeneric8)
ArrayList<ArgType> newArgTypes = new ArrayList<>(parsedArgTypes);
newArgTypes.add(0, mthArgTypes.get(0));
if (newArgTypes.size() == mthArgTypes.size()) {
return newArgTypes;
}
}
mth.addWarnComment("Incorrect args count in method signature: " + sp.getSignature());
return null;
}
for (int i = 0; i < len; i++) {
ArgType parsedType = parsedArgTypes.get(i);
ArgType mthArgType = mthArgTypes.get(i);
if (!validateParsedType(parsedType, mthArgType)) {
mth.addWarnComment("Incorrect types in method signature: " + sp.getSignature());
return null;
}
}
return parsedArgTypes;
}
private boolean validateParsedType(ArgType parsedType, ArgType currentType) {
TypeCompareEnum result = root.getTypeCompare().compareTypes(parsedType, currentType);
return result != TypeCompareEnum.CONFLICT;
}
}
jadx-core/src/test/java/jadx/tests/integration/generics/TestClassSignature.java
+1
View File
@@ -15,6 +15,7 @@ public class TestClassSignature extends SmaliTest {
@Test
public void test() {
allowWarnInCode();
assertThat(getClassNodeFromSmali())
.code()
.containsOne("Incorrect class signature")
jadx-core/src/test/java/jadx/tests/integration/generics/TestGeneric8.java
+32
View File
@@ -0,0 +1,32 @@
package jadx.tests.integration.generics;
import org.junit.jupiter.api.Test;
import jadx.tests.api.IntegrationTest;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
public class TestGeneric8 extends IntegrationTest {
public static class TestCls {
@SuppressWarnings("InnerClassMayBeStatic")
public class TestNumber<T extends Integer> {
private final T n;
public TestNumber(T n) {
this.n = n;
}
public boolean isEven() {
return n.intValue() % 2 == 0;
}
}
}
@Test
public void test() {
assertThat(getClassNode(TestCls.class))
.code()
.containsOne("public TestNumber(T n");
}
}
jadx-core/src/test/java/jadx/tests/integration/others/TestIncorrectMethodSignature.java
+22
View File
@@ -0,0 +1,22 @@
package jadx.tests.integration.others;
import org.junit.jupiter.api.Test;
import jadx.tests.api.SmaliTest;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
/**
* Issue #858.
* Incorrect method signature change argument type and shift register numbers
*/
public class TestIncorrectMethodSignature extends SmaliTest {
@Test
public void test() {
allowWarnInCode();
assertThat(getClassNodeFromSmali())
.code()
.containsOne("public TestIncorrectMethodSignature(String str) {");
}
}
jadx-core/src/test/smali/others/TestIncorrectMethodSignature.smali
+17
View File
@@ -0,0 +1,17 @@
.class public Lothers/TestIncorrectMethodSignature;
.super Ljava/lang/RuntimeException;
.source "TestIncorrectMethodSignature.java"
.method public constructor <init>(Ljava/lang/String;)V
.registers 2
.annotation system Ldalvik/annotation/Signature;
value = {
"(J)V"
}
.end annotation
invoke-direct {p0, p1}, Ljava/lang/RuntimeException;-><init>(Ljava/lang/String;)V
return-void
.end method