From 9a8ec769899c0456851ac2ca10644b63bdfc968f Mon Sep 17 00:00:00 2001 From: pubiqq <82187521+pubiqq@users.noreply.github.com> Date: Thu, 15 Aug 2024 21:59:44 +0300 Subject: [PATCH] fix: improve checking of access modifiers for classes (PR #2251) --- .../java/jadx/core/dex/nodes/RootNode.java | 7 +++ .../jadx/core/dex/nodes/utils/ClassUtils.java | 57 ++++++++++++++++++ .../core/dex/visitors/FixAccessModifiers.java | 59 ++++++++++--------- 3 files changed, 95 insertions(+), 28 deletions(-) create mode 100644 jadx-core/src/main/java/jadx/core/dex/nodes/utils/ClassUtils.java 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 index a08026bb..c24d832a 100644 --- 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.InfoStorage; 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 class RootNode { 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 class RootNode { 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 new file mode 100644 index 00000000..223f622e --- /dev/null +++ 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 index 2432025d..9e58762a 100644 --- 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.ClassNode; 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 @@ import jadx.core.utils.exceptions.JadxException; ) 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 class FixAccessModifiers extends AbstractVisitor { } 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()) { + + if (cls.isTopClass() && (accessFlags.isPrivate() || accessFlags.isProtected())) { + return AccessFlags.PUBLIC; + } + + for (ClassNode useCls : cls.getUseIn()) { + if (!classUtils.isAccessible(cls, useCls)) { 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 (accessFlags.isPackagePrivate()) { - String pkg = cls.getPackage(); - for (ClassNode useCls : cls.getUseIn()) { - if (!useCls.getPackage().equals(pkg)) { - 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 usedInClss = useMth.getUseIn().stream() + .map(MethodNode::getParentClass) + .collect(Collectors.toSet()); + + for (ClassNode useCls : usedInClss) { + if (!classUtils.isAccessible(cls, useCls)) { + return AccessFlags.PUBLIC; + } } } } + return -1; }