fix: improve checking of access modifiers for methods

skylot · Aug 15, 2024 · f9097dc · f9097dc
1 parent 9a8ec76
commit f9097dc
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 48 deletions.
diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/utils/AccessCheckUtils.java b/jadx-core/src/main/java/jadx/core/dex/nodes/utils/AccessCheckUtils.java
@@ -0,0 +1,85 @@
+package jadx.core.dex.nodes.utils;
+
+import jadx.core.dex.info.AccessInfo;
+import jadx.core.dex.nodes.ClassNode;
+import jadx.core.dex.nodes.FieldNode;
+import jadx.core.dex.nodes.ICodeNode;
+import jadx.core.dex.nodes.MethodNode;
+import jadx.core.dex.nodes.RootNode;
+import jadx.core.utils.exceptions.JadxRuntimeException;
+
+class AccessCheckUtils {
+
+	private final RootNode root;
+
+	AccessCheckUtils(RootNode rootNode) {
+		this.root = rootNode;
+	}
+
+	boolean isAccessible(ICodeNode targetNode, ICodeNode callerNode) {
+		ClassNode targetCls = getDeclaringClass(targetNode);
+		ClassNode callerCls = getDeclaringClass(callerNode);
+
+		if (targetCls.equals(callerCls)) {
+			return true;
+		}
+
+		AccessInfo targetVisibility;
+		if (targetNode == targetCls) {
+			targetVisibility = targetNode.getAccessFlags().getVisibility();
+		} else {
+			AccessInfo targetClsVisibility = targetCls.getAccessFlags().getVisibility();
+			AccessInfo targetNodeVisibility = targetNode.getAccessFlags().getVisibility();
+			targetVisibility = targetClsVisibility.isVisibilityWeakerThan(targetNodeVisibility)
+					? targetClsVisibility
+					: targetNodeVisibility;
+		}
+
+		if (targetVisibility.isPublic()) {
+			return true;
+		}
+
+		if (targetVisibility.isProtected()) {
+			return isProtectedAccessible(targetCls, callerCls);
+		}
+
+		if (targetVisibility.isPackagePrivate()) {
+			return isPackagePrivateAccessible(targetCls, callerCls);
+		}
+
+		if (targetVisibility.isPrivate()) {
+			return isPrivateAccessible(targetCls, callerCls);
+		}
+
+		throw new JadxRuntimeException(targetVisibility + " is not supported");
+	}
+
+	private ClassNode getDeclaringClass(ICodeNode node) {
+		if (node instanceof ClassNode) {
+			return (ClassNode) node;
+		} else if (node instanceof MethodNode) {
+			return ((MethodNode) node).getParentClass();
+		} else if (node instanceof FieldNode) {
+			return ((FieldNode) node).getParentClass();
+		} else {
+			throw new JadxRuntimeException(node + " is not supported");
+		}
+	}
+
+	private boolean isProtectedAccessible(ClassNode cls, ClassNode callerCls) {
+		return isPackagePrivateAccessible(cls, callerCls) || isSuperType(cls, callerCls);
+	}
+
+	private boolean isPackagePrivateAccessible(ClassNode cls, ClassNode callerCls) {
+		return cls.getPackageNode().equals(callerCls.getPackageNode());
+	}
+
+	private boolean isPrivateAccessible(ClassNode cls, ClassNode callerCls) {
+		return cls.getTopParentClass().equals(callerCls.getTopParentClass());
+	}
+
+	private boolean isSuperType(ClassNode cls, ClassNode superCls) {
+		return root.getClsp().getSuperTypes(cls.getRawName()).stream()
+				.anyMatch(x -> x.equals(superCls.getRawName()));
+	}
+}
diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/utils/ClassUtils.java b/jadx-core/src/main/java/jadx/core/dex/nodes/utils/ClassUtils.java
@@ -1,57 +1,19 @@
 package jadx.core.dex.nodes.utils;
 
-import jadx.core.dex.info.AccessInfo;
 import jadx.core.dex.nodes.ClassNode;
 import jadx.core.dex.nodes.RootNode;
-import jadx.core.utils.exceptions.JadxRuntimeException;
 
 public class ClassUtils {
 
 	private final RootNode root;
+	private final AccessCheckUtils accessCheckUtils;
 
 	public ClassUtils(RootNode rootNode) {
 		this.root = rootNode;
+		this.accessCheckUtils = new AccessCheckUtils(root);
 	}
 
 	public boolean isAccessible(ClassNode cls, ClassNode callerCls) {
-		if (cls.equals(callerCls)) {
-			return true;
-		}
-
-		final AccessInfo accessFlags = cls.getAccessFlags();
-		if (accessFlags.isPublic()) {
-			return true;
-		}
-
-		if (accessFlags.isProtected()) {
-			return isProtectedAccessible(cls, callerCls);
-		}
-
-		if (accessFlags.isPackagePrivate()) {
-			return isPackagePrivateAccessible(cls, callerCls);
-		}
-
-		if (accessFlags.isPrivate()) {
-			return isPrivateAccessible(cls, callerCls);
-		}
-
-		throw new JadxRuntimeException(accessFlags + " is not supported");
-	}
-
-	private boolean isProtectedAccessible(ClassNode cls, ClassNode callerCls) {
-		return isPackagePrivateAccessible(cls, callerCls) || isSuperType(cls, callerCls);
-	}
-
-	private boolean isPackagePrivateAccessible(ClassNode cls, ClassNode callerCls) {
-		return cls.getPackageNode().equals(callerCls.getPackageNode());
-	}
-
-	private boolean isPrivateAccessible(ClassNode cls, ClassNode callerCls) {
-		return cls.getTopParentClass().equals(callerCls.getTopParentClass());
-	}
-
-	private boolean isSuperType(ClassNode cls, ClassNode superCls) {
-		return root.getClsp().getSuperTypes(cls.getRawName()).stream()
-				.anyMatch(x -> x.equals(superCls.getRawName()));
+		return accessCheckUtils.isAccessible(cls, callerCls);
 	}
 }
diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/utils/MethodUtils.java b/jadx-core/src/main/java/jadx/core/dex/nodes/utils/MethodUtils.java
@@ -24,9 +24,11 @@
 
 public class MethodUtils {
 	private final RootNode root;
+	private final AccessCheckUtils accessCheckUtils;
 
 	public MethodUtils(RootNode rootNode) {
 		this.root = rootNode;
+		this.accessCheckUtils = new AccessCheckUtils(root);
 	}
 
 	@Nullable
@@ -171,4 +173,8 @@ public ClassInfo getMethodOriginDeclClass(MethodNode mth) {
 		}
 		return mth.getMethodInfo().getDeclClass();
 	}
+
+	public boolean isAccessible(MethodNode mth, MethodNode callerMth) {
+		return accessCheckUtils.isAccessible(mth, callerMth);
+	}
 }
diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java b/jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java
@@ -15,6 +15,7 @@
 import jadx.core.dex.nodes.MethodNode;
 import jadx.core.dex.nodes.RootNode;
 import jadx.core.dex.nodes.utils.ClassUtils;
+import jadx.core.dex.nodes.utils.MethodUtils;
 import jadx.core.utils.exceptions.JadxException;
 
 @JadxVisitor(
@@ -25,12 +26,14 @@
 public class FixAccessModifiers extends AbstractVisitor {
 
 	private ClassUtils classUtils;
+	private MethodUtils methodUtils;
 
 	private boolean respectAccessModifiers;
 
 	@Override
 	public void init(RootNode root) {
 		this.classUtils = root.getClassUtils();
+		this.methodUtils = root.getMethodUtils();
 		this.respectAccessModifiers = root.getArgs().isRespectBytecodeAccModifiers();
 	}
 
@@ -103,11 +106,12 @@ private int fixClassVisibility(ClassNode cls) {
 		return -1;
 	}
 
-	private static int fixMethodVisibility(MethodNode mth) {
+	private int fixMethodVisibility(MethodNode mth) {
 		AccessInfo accessFlags = mth.getAccessFlags();
 		if (accessFlags.isPublic()) {
 			return -1;
 		}
+
 		MethodOverrideAttr overrideAttr = mth.get(AType.METHOD_OVERRIDE);
 		if (overrideAttr != null && !overrideAttr.getOverrideList().isEmpty()) {
 			// visibility can't be weaker
@@ -117,17 +121,13 @@ private static int fixMethodVisibility(MethodNode mth) {
 				return parentAccInfo.getVisibility().rawValue();
 			}
 		}
-		if (mth.getUseIn().isEmpty()) {
-			return -1;
-		}
 
-		ClassNode thisTopParentCls = mth.getParentClass().getTopParentClass();
 		for (MethodNode useMth : mth.getUseIn()) {
-			ClassNode useInTPCls = useMth.getParentClass().getTopParentClass();
-			if (!useInTPCls.equals(thisTopParentCls)) {
+			if (!methodUtils.isAccessible(mth, useMth)) {
 				return AccessFlags.PUBLIC;
 			}
 		}
+
 		return -1;
 	}
 }