From ffdad1b6525999d0a307527c98d90d7fd49614fd Mon Sep 17 00:00:00 2001 From: pubiqq <82187521+pubiqq@users.noreply.github.com> Date: Fri, 16 Aug 2024 01:20:07 +0300 Subject: [PATCH] fix: improve checking of access modifiers for methods (PR #2252) --- .../dex/nodes/utils/AccessCheckUtils.java | 85 +++++++++++++++++++ .../jadx/core/dex/nodes/utils/ClassUtils.java | 44 +--------- .../core/dex/nodes/utils/MethodUtils.java | 6 ++ .../core/dex/visitors/FixAccessModifiers.java | 14 +-- 4 files changed, 101 insertions(+), 48 deletions(-) create mode 100644 jadx-core/src/main/java/jadx/core/dex/nodes/utils/AccessCheckUtils.java 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 new file mode 100644 index 000000000..e828f58a5 --- /dev/null +++ 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 index 223f622e5..8dd006943 100644 --- 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 index 3e21681f3..837e8e260 100644 --- 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 @@ import jadx.core.utils.Utils; 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 class MethodUtils { } 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 index 9e58762a6..db0fde306 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 @@ -15,6 +15,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.dex.nodes.utils.MethodUtils; import jadx.core.utils.exceptions.JadxException; @JadxVisitor( @@ -25,12 +26,14 @@ import jadx.core.utils.exceptions.JadxException; 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 @@ public class FixAccessModifiers extends AbstractVisitor { 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 @@ public class FixAccessModifiers extends AbstractVisitor { 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; } }