fix: improve checking of access modifiers for classes (PR #2251)

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

Hide file tree

Showing 3 changed files with 96 additions and 29 deletions.
diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java
@@ -40,6 +40,7 @@
 import jadx.core.dex.info.MethodInfo;
 import jadx.core.dex.info.PackageInfo;
 import jadx.core.dex.instructions.args.ArgType;
+import jadx.core.dex.nodes.utils.ClassUtils;
 import jadx.core.dex.nodes.utils.MethodUtils;
 import jadx.core.dex.nodes.utils.TypeUtils;
 import jadx.core.dex.visitors.DepthTraversal;
@@ -71,6 +72,7 @@ public class RootNode {
 	private final InfoStorage infoStorage = new InfoStorage();
 	private final CacheStorage cacheStorage = new CacheStorage();
 	private final TypeUpdate typeUpdate;
+	private final ClassUtils classUtils;
 	private final MethodUtils methodUtils;
 	private final TypeUtils typeUtils;
 	private final AttributeStorage attributes = new AttributeStorage();
@@ -104,6 +106,7 @@ public RootNode(JadxArgs args) {
 		this.stringUtils = new StringUtils(args);
 		this.constValues = new ConstStorage(args);
 		this.typeUpdate = new TypeUpdate(this);
+		this.classUtils = new ClassUtils(this);
 		this.methodUtils = new MethodUtils(this);
 		this.typeUtils = new TypeUtils(this);
 	}
@@ -704,6 +707,10 @@ public ICodeCache getCodeCache() {
 		return args.getCodeCache();
 	}
 
+	public ClassUtils getClassUtils() {
+		return classUtils;
+	}
+
 	public MethodUtils getMethodUtils() {
 		return methodUtils;
 	}

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
@@ -0,0 +1,57 @@
+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;
+
+	public ClassUtils(RootNode rootNode) {
+		this.root = rootNode;
+	}
+
+	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()));
+	}
+}
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
@@ -1,5 +1,8 @@
 package jadx.core.dex.visitors;
 
+import java.util.Set;
+import java.util.stream.Collectors;
+
 import jadx.api.plugins.input.data.AccessFlags;
 import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.attributes.AType;
@@ -11,6 +14,7 @@
 import jadx.core.dex.nodes.IMethodDetails;
 import jadx.core.dex.nodes.MethodNode;
 import jadx.core.dex.nodes.RootNode;
+import jadx.core.dex.nodes.utils.ClassUtils;
 import jadx.core.utils.exceptions.JadxException;
 
 @JadxVisitor(
@@ -20,10 +24,13 @@
 )
 public class FixAccessModifiers extends AbstractVisitor {
 
+	private ClassUtils classUtils;
+
 	private boolean respectAccessModifiers;
 
 	@Override
 	public void init(RootNode root) {
+		this.classUtils = root.getClassUtils();
 		this.respectAccessModifiers = root.getArgs().isRespectBytecodeAccModifiers();
 	}
 
@@ -60,43 +67,39 @@ public static void changeVisibility(NotificationAttrNode node, int newVisFlag) {
 	}
 
 	private int fixClassVisibility(ClassNode cls) {
-		if (cls.getUseIn().isEmpty()) {
+		AccessInfo accessFlags = cls.getAccessFlags();
+		if (accessFlags.isPublic()) {
 			return -1;
 		}
-		AccessInfo accessFlags = cls.getAccessFlags();
-		if (accessFlags.isPrivate()) {
-			if (!cls.isInner()) {
-				return AccessFlags.PUBLIC;
-			}
-			// check if private inner class is used outside
-			ClassNode topParentClass = cls.getTopParentClass();
-			for (ClassNode useCls : cls.getUseIn()) {
-				if (useCls.getTopParentClass() != topParentClass) {
-					return AccessFlags.PUBLIC;
-				}
-			}
+
+		if (cls.isTopClass() && (accessFlags.isPrivate() || accessFlags.isProtected())) {
+			return AccessFlags.PUBLIC;
 		}
-		if (accessFlags.isPackagePrivate()) {
-			String pkg = cls.getPackage();
-			for (ClassNode useCls : cls.getUseIn()) {
-				if (!useCls.getPackage().equals(pkg)) {
-					return AccessFlags.PUBLIC;
-				}
+
+		for (ClassNode useCls : cls.getUseIn()) {
+			if (!classUtils.isAccessible(cls, useCls)) {
+				return AccessFlags.PUBLIC;
 			}
 		}
-		if (!accessFlags.isPublic()) {
-			// if class is used in inlinable method => make it public
-			for (MethodNode useMth : cls.getUseInMth()) {
-				MethodInlineAttr inlineAttr = useMth.get(AType.METHOD_INLINE);
-				boolean isInline = inlineAttr != null && !inlineAttr.notNeeded();
-				boolean isCandidateForInline = useMth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE);
-
-				boolean mostLikelyInline = isInline || isCandidateForInline;
-				if (mostLikelyInline && !useMth.getUseIn().isEmpty()) {
-					return AccessFlags.PUBLIC;
+
+		for (MethodNode useMth : cls.getUseInMth()) {
+			MethodInlineAttr inlineAttr = useMth.get(AType.METHOD_INLINE);
+			boolean isInline = inlineAttr != null && !inlineAttr.notNeeded();
+			boolean isCandidateForInline = useMth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE);
+
+			if (isInline || isCandidateForInline) {
+				Set<ClassNode> usedInClss = useMth.getUseIn().stream()
+						.map(MethodNode::getParentClass)
+						.collect(Collectors.toSet());
+
+				for (ClassNode useCls : usedInClss) {
+					if (!classUtils.isAccessible(cls, useCls)) {
+						return AccessFlags.PUBLIC;
+					}
 				}
 			}
 		}
+
 		return -1;
 	}