fix: check full signature for search method override (#1743)

jadx-core/src/main/java/jadx/core/dex/visitors/OverrideMethodVisitor.java

@@ -90,7 +90,7 @@ public class OverrideMethodVisitor extends AbstractVisitor {
 		for (ArgType superType : superData.getSuperTypes()) {
 			ClassNode classNode = mth.root().resolveClass(superType);
 			if (classNode != null) {
-				MethodNode ovrdMth = searchOverriddenMethod(classNode, signature);
+				MethodNode ovrdMth = searchOverriddenMethod(classNode, mth, signature);
 				if (ovrdMth != null) {
 					if (isMethodVisibleInCls(ovrdMth, cls)) {
 						overrideList.add(ovrdMth);
@@ -107,6 +107,8 @@ public class OverrideMethodVisitor extends AbstractVisitor {
 					Map<String, ClspMethod> methodsMap = clsDetails.getMethodsMap();
 					for (Map.Entry<String, ClspMethod> entry : methodsMap.entrySet()) {
 						String mthShortId = entry.getKey();
+						// do not check full signature, classpath methods can be trusted
+						// i.e. doesn't contain methods with same signature in one class
 						if (mthShortId.startsWith(signature)) {
 							overrideList.add(entry.getValue());
 							break;
@@ -130,12 +132,30 @@ public class OverrideMethodVisitor extends AbstractVisitor {
 	}
 
 	@Nullable
-	private MethodNode searchOverriddenMethod(ClassNode cls, String signature) {
+	private MethodNode searchOverriddenMethod(ClassNode cls, MethodNode mth, String signature) {
+		// search by exact full signature (with return value) to fight obfuscation (see test
+		// 'TestOverrideWithSameName')
+		String shortId = mth.getMethodInfo().getShortId();
 		for (MethodNode supMth : cls.getMethods()) {
-			if (!supMth.getAccessFlags().isStatic() && supMth.getMethodInfo().getShortId().startsWith(signature)) {
+			if (supMth.getMethodInfo().getShortId().equals(shortId) && !supMth.getAccessFlags().isStatic()) {
 				return supMth;
 			}
 		}
+		// search by signature without return value and check if return value is wider type
+		for (MethodNode supMth : cls.getMethods()) {
+			if (supMth.getMethodInfo().getShortId().startsWith(signature) && !supMth.getAccessFlags().isStatic()) {
+				TypeCompare typeCompare = cls.root().getTypeCompare();
+				ArgType supRetType = supMth.getMethodInfo().getReturnType();
+				ArgType mthRetType = mth.getMethodInfo().getReturnType();
+				TypeCompareEnum res = typeCompare.compareTypes(supRetType, mthRetType);
+				if (res.isWider()) {
+					return supMth;
+				}
+				if (res == TypeCompareEnum.UNKNOWN || res == TypeCompareEnum.CONFLICT) {
+					mth.addDebugComment("Possible override for method " + supMth.getMethodInfo().getFullId());
+				}
+			}
+		}
 		return null;
 	}
 

jadx-core/src/test/java/jadx/tests/integration/others/TestOverrideWithSameName.java

+package jadx.tests.integration.others;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.core.dex.attributes.AType;
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
+
+@SuppressWarnings("CommentedOutCode")
+public class TestOverrideWithSameName extends SmaliTest {
+
+	//@formatter:off
+	/*
+		interface A {
+			B a();
+			C a();
+		}
+
+		abstract class B implements A {
+			@Override
+			public C a() {
+				return null;
+			}
+		}
+
+		public class C extends B {
+			@Override
+			public B a() {
+				return null;
+			}
+		}
+	*/
+	//@formatter:on
+
+	@Test
+	public void test() {
+		List<ClassNode> clsNodes = loadFromSmaliFiles();
+		assertThat(searchCls(clsNodes, "test.A"))
+				.code()
+				.containsOne("C mo0a();") // assume second method was renamed
+				.doesNotContain("@Override");
+
+		ClassNode bCls = searchCls(clsNodes, "test.B");
+		assertThat(bCls)
+				.code()
+				.containsOne("C mo0a() {")
+				.containsOne("@Override");
+
+		assertThat(getMethod(bCls, "a").get(AType.METHOD_OVERRIDE).getOverrideList())
+				.singleElement()
+				.satisfies(mth -> assertThat(mth.getMethodInfo().getDeclClass().getShortName()).isEqualTo("A"));
+
+		ClassNode cCls = searchCls(clsNodes, "test.C");
+		assertThat(cCls)
+				.code()
+				.containsOne("B a() {")
+				.containsOne("@Override");
+
+		assertThat(getMethod(cCls, "a").get(AType.METHOD_OVERRIDE).getOverrideList())
+				.singleElement()
+				.satisfies(mth -> assertThat(mth.getMethodInfo().getDeclClass().getShortName()).isEqualTo("A"));
+	}
+}

jadx-core/src/test/smali/others/TestOverrideWithSameName/A.smali

+.class interface abstract Ltest/A;
+.super Ljava/lang/Object;
+
+.method public abstract a()Ltest/B;
+.end method
+
+.method public abstract a()Ltest/C;
+.end method

jadx-core/src/test/smali/others/TestOverrideWithSameName/B.smali

+.class abstract Ltest/B;
+.super Ljava/lang/Object;
+
+.implements Ltest/A;
+
+.method public a()Ltest/C;
+    .registers 2
+    const/4 v0, 0x0
+    return-object v0
+.end method

jadx-core/src/test/smali/others/TestOverrideWithSameName/C.smali

+.class public Ltest/C;
+.super Ltest/B;
+
+.method public a()Ltest/B;
+    .registers 2
+    const/4 v0, 0x0
+    return-object v0
+.end method