fix: add more checks before remove or rename enum methods (#1572)
This commit is contained in:
@@ -823,7 +823,9 @@ public class InsnGen {
|
||||
}
|
||||
if (callMthNode != null) {
|
||||
code.attachAnnotation(callMthNode);
|
||||
code.add(callMthNode.getAlias());
|
||||
}
|
||||
if (insn.contains(AFlag.FORCE_RAW_NAME)) {
|
||||
code.add(callMth.getName());
|
||||
} else {
|
||||
code.add(callMth.getAlias());
|
||||
}
|
||||
|
||||
@@ -25,6 +25,8 @@ public enum AFlag {
|
||||
HIDDEN, // instruction used inside other instruction but not listed in args
|
||||
|
||||
DONT_RENAME, // do not rename during deobfuscation
|
||||
FORCE_RAW_NAME, // force use of raw name instead alias
|
||||
|
||||
ADDED_TO_REGION,
|
||||
|
||||
EXC_TOP_SPLITTER,
|
||||
|
||||
@@ -18,6 +18,7 @@ import jadx.core.dex.attributes.AFlag;
|
||||
import jadx.core.dex.attributes.AType;
|
||||
import jadx.core.dex.attributes.nodes.EnumClassAttr;
|
||||
import jadx.core.dex.attributes.nodes.EnumClassAttr.EnumField;
|
||||
import jadx.core.dex.attributes.nodes.RenameReasonAttr;
|
||||
import jadx.core.dex.attributes.nodes.SkipMethodArgsAttr;
|
||||
import jadx.core.dex.info.AccessInfo;
|
||||
import jadx.core.dex.info.ClassInfo;
|
||||
@@ -26,6 +27,7 @@ import jadx.core.dex.info.MethodInfo;
|
||||
import jadx.core.dex.instructions.IndexInsnNode;
|
||||
import jadx.core.dex.instructions.InsnType;
|
||||
import jadx.core.dex.instructions.InvokeNode;
|
||||
import jadx.core.dex.instructions.InvokeType;
|
||||
import jadx.core.dex.instructions.args.ArgType;
|
||||
import jadx.core.dex.instructions.args.InsnArg;
|
||||
import jadx.core.dex.instructions.args.InsnWrapArg;
|
||||
@@ -59,6 +61,7 @@ import static jadx.core.utils.InsnUtils.getWrappedInsn;
|
||||
public class EnumVisitor extends AbstractVisitor {
|
||||
|
||||
private MethodInfo enumValueOfMth;
|
||||
private MethodInfo cloneMth;
|
||||
|
||||
@Override
|
||||
public void init(RootNode root) {
|
||||
@@ -68,6 +71,12 @@ public class EnumVisitor extends AbstractVisitor {
|
||||
"valueOf",
|
||||
Arrays.asList(ArgType.CLASS, ArgType.STRING),
|
||||
ArgType.ENUM);
|
||||
|
||||
cloneMth = MethodInfo.fromDetails(root,
|
||||
ClassInfo.fromType(root, ArgType.OBJECT),
|
||||
"clone",
|
||||
Collections.emptyList(),
|
||||
ArgType.OBJECT);
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -377,6 +386,7 @@ public class EnumVisitor extends AbstractVisitor {
|
||||
return enumFieldNode;
|
||||
}
|
||||
|
||||
@SuppressWarnings("StatementWithEmptyBody")
|
||||
private EnumField createEnumFieldByConstructor(ClassNode cls, FieldNode enumFieldNode, ConstructorInsn co) {
|
||||
// usually constructor signature is '<init>(Ljava/lang/String;I)V'.
|
||||
// sometimes for one field enum second arg can be omitted
|
||||
@@ -417,13 +427,12 @@ public class EnumVisitor extends AbstractVisitor {
|
||||
}
|
||||
|
||||
private void removeEnumMethods(ClassNode cls, ArgType clsType, FieldNode valuesField) {
|
||||
String valuesMethod = "values()" + TypeGen.signature(ArgType.array(clsType));
|
||||
FieldInfo valuesFieldInfo = valuesField.getFieldInfo();
|
||||
|
||||
String valuesMethodShortId = "values()" + TypeGen.signature(ArgType.array(clsType));
|
||||
MethodNode valuesMethod = null;
|
||||
// remove compiler generated methods
|
||||
for (MethodNode mth : cls.getMethods()) {
|
||||
MethodInfo mi = mth.getMethodInfo();
|
||||
if (mi.isClassInit()) {
|
||||
if (mi.isClassInit() || mth.isNoCode()) {
|
||||
continue;
|
||||
}
|
||||
String shortId = mi.getShortId();
|
||||
@@ -432,12 +441,33 @@ public class EnumVisitor extends AbstractVisitor {
|
||||
mth.add(AFlag.DONT_GENERATE);
|
||||
}
|
||||
markArgsForSkip(mth);
|
||||
} else if (shortId.equals(valuesMethod)
|
||||
|| usesValuesField(mth, valuesFieldInfo)
|
||||
|| simpleValueOfMth(mth, clsType)) {
|
||||
} else if (mi.getShortId().equals(valuesMethodShortId)) {
|
||||
if (isValuesMethod(mth, clsType)) {
|
||||
valuesMethod = mth;
|
||||
mth.add(AFlag.DONT_GENERATE);
|
||||
} else {
|
||||
// custom values method => rename to resolve conflict with enum method
|
||||
mth.getMethodInfo().setAlias("valuesCustom");
|
||||
mth.addAttr(new RenameReasonAttr(mth).append("to resolve conflict with enum method"));
|
||||
}
|
||||
} else if (isValuesMethod(mth, clsType)) {
|
||||
if (!mth.getMethodInfo().getAlias().equals("values") && !mth.getUseIn().isEmpty()) {
|
||||
// rename to use default values method
|
||||
mth.getMethodInfo().setAlias("values");
|
||||
mth.addAttr(new RenameReasonAttr(mth).append("to match enum method name"));
|
||||
mth.add(AFlag.DONT_RENAME);
|
||||
}
|
||||
valuesMethod = mth;
|
||||
mth.add(AFlag.DONT_GENERATE);
|
||||
} else if (simpleValueOfMth(mth, clsType)) {
|
||||
mth.add(AFlag.DONT_GENERATE);
|
||||
}
|
||||
}
|
||||
FieldInfo valuesFieldInfo = valuesField.getFieldInfo();
|
||||
for (MethodNode mth : cls.getMethods()) {
|
||||
// fix access to 'values' field and 'values()' method
|
||||
fixValuesAccess(mth, valuesFieldInfo, clsType, valuesMethod);
|
||||
}
|
||||
}
|
||||
|
||||
private void markArgsForSkip(MethodNode mth) {
|
||||
@@ -458,6 +488,25 @@ public class EnumVisitor extends AbstractVisitor {
|
||||
return false;
|
||||
}
|
||||
|
||||
// TODO: support other method patterns ???
|
||||
private boolean isValuesMethod(MethodNode mth, ArgType clsType) {
|
||||
ArgType retType = mth.getReturnType();
|
||||
if (!retType.isArray() || !retType.getArrayElement().equals(clsType)) {
|
||||
return false;
|
||||
}
|
||||
InsnNode returnInsn = BlockUtils.getOnlyOneInsnFromMth(mth);
|
||||
if (returnInsn == null || returnInsn.getType() != InsnType.RETURN || returnInsn.getArgsCount() != 1) {
|
||||
return false;
|
||||
}
|
||||
InsnNode wrappedInsn = getWrappedInsn(getSingleArg(returnInsn));
|
||||
IndexInsnNode castInsn = (IndexInsnNode) checkInsnType(wrappedInsn, InsnType.CHECK_CAST);
|
||||
if (castInsn != null && Objects.equals(castInsn.getIndex(), ArgType.array(clsType))) {
|
||||
InvokeNode invokeInsn = (InvokeNode) checkInsnType(getWrappedInsn(getSingleArg(castInsn)), InsnType.INVOKE);
|
||||
return invokeInsn != null && invokeInsn.getCallMth().equals(cloneMth);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private boolean simpleValueOfMth(MethodNode mth, ArgType clsType) {
|
||||
InsnNode returnInsn = InsnUtils.searchSingleReturnInsn(mth, insn -> insn.getArgsCount() == 1);
|
||||
if (returnInsn == null) {
|
||||
@@ -472,9 +521,41 @@ public class EnumVisitor extends AbstractVisitor {
|
||||
return false;
|
||||
}
|
||||
|
||||
private boolean usesValuesField(MethodNode mth, FieldInfo valuesFieldInfo) {
|
||||
private void fixValuesAccess(MethodNode mth, FieldInfo valuesFieldInfo, ArgType clsType, @Nullable MethodNode valuesMethod) {
|
||||
MethodInfo mi = mth.getMethodInfo();
|
||||
if (mi.isConstructor() || mi.isClassInit() || mth.isNoCode() || mth == valuesMethod) {
|
||||
return;
|
||||
}
|
||||
// search value field usage
|
||||
Predicate<InsnNode> insnTest = insn -> Objects.equals(((IndexInsnNode) insn).getIndex(), valuesFieldInfo);
|
||||
return InsnUtils.searchInsn(mth, InsnType.SGET, insnTest) != null;
|
||||
InsnNode useInsn = InsnUtils.searchInsn(mth, InsnType.SGET, insnTest);
|
||||
if (useInsn == null) {
|
||||
return;
|
||||
}
|
||||
// replace 'values' field with 'values()' method
|
||||
InsnUtils.replaceInsns(mth, insn -> {
|
||||
if (insn.getType() == InsnType.SGET && insnTest.test(insn)) {
|
||||
MethodInfo valueMth = valuesMethod == null
|
||||
? getValueMthInfo(mth.root(), clsType)
|
||||
: valuesMethod.getMethodInfo();
|
||||
InvokeNode invokeNode = new InvokeNode(valueMth, InvokeType.STATIC, 0);
|
||||
invokeNode.setResult(insn.getResult());
|
||||
if (valuesMethod == null) {
|
||||
// forcing enum method (can overlap and get renamed by custom method)
|
||||
invokeNode.add(AFlag.FORCE_RAW_NAME);
|
||||
}
|
||||
mth.addDebugComment("Replace access to removed values field (" + valuesFieldInfo.getName() + ") with 'values()' method");
|
||||
return invokeNode;
|
||||
}
|
||||
return null;
|
||||
});
|
||||
}
|
||||
|
||||
private MethodInfo getValueMthInfo(RootNode root, ArgType clsType) {
|
||||
return MethodInfo.fromDetails(root,
|
||||
ClassInfo.fromType(root, clsType),
|
||||
"values",
|
||||
Collections.emptyList(), ArgType.array(clsType));
|
||||
}
|
||||
|
||||
private static void processEnumCls(ClassNode cls, EnumField field, ClassNode innerCls) {
|
||||
|
||||
@@ -1013,6 +1013,9 @@ public class BlockUtils {
|
||||
*/
|
||||
@Nullable
|
||||
public static InsnNode getOnlyOneInsnFromMth(MethodNode mth) {
|
||||
if (mth.isNoCode()) {
|
||||
return null;
|
||||
}
|
||||
InsnNode insn = null;
|
||||
for (BlockNode block : mth.getBasicBlocks()) {
|
||||
List<InsnNode> blockInsns = block.getInstructions();
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package jadx.core.utils;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.function.Function;
|
||||
import java.util.function.Predicate;
|
||||
|
||||
import org.jetbrains.annotations.Nullable;
|
||||
@@ -140,6 +141,37 @@ public class InsnUtils {
|
||||
return null;
|
||||
}
|
||||
|
||||
public static void replaceInsns(MethodNode mth, Function<InsnNode, InsnNode> replaceFunction) {
|
||||
for (BlockNode block : mth.getBasicBlocks()) {
|
||||
List<InsnNode> insns = block.getInstructions();
|
||||
int insnsCount = insns.size();
|
||||
for (int i = 0; i < insnsCount; i++) {
|
||||
InsnNode insn = insns.get(i);
|
||||
replaceInsnsInInsn(mth, insn, replaceFunction);
|
||||
InsnNode replace = replaceFunction.apply(insn);
|
||||
if (replace != null) {
|
||||
BlockUtils.replaceInsn(mth, block, i, replace);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public static void replaceInsnsInInsn(MethodNode mth, InsnNode insn, Function<InsnNode, InsnNode> replaceFunction) {
|
||||
int argsCount = insn.getArgsCount();
|
||||
for (int i = 0; i < argsCount; i++) {
|
||||
InsnArg arg = insn.getArg(i);
|
||||
if (arg.isInsnWrap()) {
|
||||
InsnNode wrapInsn = ((InsnWrapArg) arg).getWrapInsn();
|
||||
replaceInsnsInInsn(mth, wrapInsn, replaceFunction);
|
||||
InsnNode replace = replaceFunction.apply(wrapInsn);
|
||||
if (replace != null) {
|
||||
InsnRemover.unbindArgUsage(mth, arg);
|
||||
insn.setArg(i, InsnArg.wrapInsnIntoArg(replace));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Nullable
|
||||
public static RegisterArg getRegFromInsn(List<RegisterArg> regs, InsnType insnType) {
|
||||
for (RegisterArg reg : regs) {
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
package jadx.tests.integration.enums;
|
||||
|
||||
import java.util.Collections;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import jadx.core.dex.nodes.ClassNode;
|
||||
import jadx.api.CommentsLevel;
|
||||
import jadx.tests.api.SmaliTest;
|
||||
|
||||
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
|
||||
@@ -32,14 +34,31 @@ public class TestEnumObfuscated extends SmaliTest {
|
||||
public synthetic int getNum() {
|
||||
return this.num;
|
||||
}
|
||||
|
||||
// custom values method
|
||||
// should be kept and renamed to avoid collision to enum 'values()' method
|
||||
public static int values() {
|
||||
return new TestEnumObfuscated[0];
|
||||
}
|
||||
|
||||
// usage of renamed 'values()' method, should be renamed back to 'values'
|
||||
public static int valuesCount() {
|
||||
return vs().length;
|
||||
}
|
||||
|
||||
// usage of renamed '$VALUES' field, should be replaced with 'values()' method call
|
||||
public static int valuesFieldUse() {
|
||||
return $VLS.length;
|
||||
}
|
||||
}
|
||||
*/
|
||||
// @formatter:on
|
||||
|
||||
@Test
|
||||
public void test() {
|
||||
ClassNode cls = getClassNodeFromSmali();
|
||||
assertThat(cls)
|
||||
getArgs().setCommentsLevel(CommentsLevel.WARN);
|
||||
getArgs().setRenameFlags(Collections.emptySet());
|
||||
assertThat(getClassNodeFromSmali())
|
||||
.code()
|
||||
.doesNotContain("$VLS")
|
||||
.doesNotContain("vo(")
|
||||
|
||||
@@ -72,6 +72,27 @@
|
||||
return-object v0
|
||||
.end method
|
||||
|
||||
.method public static values()[Lenums/TestEnumObfuscated;
|
||||
.registers 1
|
||||
sget-object v0, Lenums/TestEnumObfuscated;->$VLS:[Lenums/TestEnumObfuscated;
|
||||
return v0
|
||||
.end method
|
||||
|
||||
.method public static valuesCount()I
|
||||
.registers 2
|
||||
invoke-static {v0, p0}, Lenums/TestEnumObfuscated;->vs()[Lenums/TestEnumObfuscated;
|
||||
move-result-object v0
|
||||
array-length v1, v0
|
||||
return v1
|
||||
.end method
|
||||
|
||||
.method public static valuesFieldUse()I
|
||||
.registers 2
|
||||
sget-object v0, Lenums/TestEnumObfuscated;->$VLS:[Lenums/TestEnumObfuscated;
|
||||
array-length v1, v0
|
||||
return v1
|
||||
.end method
|
||||
|
||||
.method public synthetic getNum()I
|
||||
.registers 2
|
||||
|
||||
|
||||
Reference in New Issue
Block a user