You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by ku...@apache.org on 2023/11/03 07:11:08 UTC
(struts) branch WW-5350-allowlist created (now ff81928d8)
This is an automated email from the ASF dual-hosted git repository.
kusal pushed a change to branch WW-5350-allowlist
in repository https://gitbox.apache.org/repos/asf/struts.git
at ff81928d8 WW-5350 Refactor SecurityMemberAccess (changes semantics)
This branch includes the following new commits:
new ff81928d8 WW-5350 Refactor SecurityMemberAccess (changes semantics)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(struts) 01/01: WW-5350 Refactor SecurityMemberAccess (changes semantics)
Posted by ku...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kusal pushed a commit to branch WW-5350-allowlist
in repository https://gitbox.apache.org/repos/asf/struts.git
commit ff81928d8c2e47ed351a582e8614734a676ed409
Author: Kusal Kithul-Godage <gi...@kusal.io>
AuthorDate: Fri Nov 3 18:10:57 2023 +1100
WW-5350 Refactor SecurityMemberAccess (changes semantics)
---
.../xwork2/ognl/SecurityMemberAccess.java | 136 +++++++++++++--------
1 file changed, 85 insertions(+), 51 deletions(-)
diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
index 232c33e5a..cdfc90306 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
@@ -104,89 +104,119 @@ public class SecurityMemberAccess implements MemberAccess {
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
- final int memberModifiers = member.getModifiers();
- final Class<?> memberClass = member.getDeclaringClass();
- // target can be null in case of accessing static fields, since OGNL 3.2.8
- final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass();
- if (!memberClass.isAssignableFrom(targetClass)) {
+ if (target != null && !member.getDeclaringClass().isAssignableFrom(target.getClass())) {
throw new IllegalArgumentException("Target does not match member!");
}
- if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
- LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", targetClass, target, member);
+ if (!checkProxyMemberAccess(target, member)) {
+ LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", target.getClass(), target, member);
return false;
}
- if (!checkPublicMemberAccess(memberModifiers)) {
+ if (!checkPublicMemberAccess(member.getModifiers())) {
LOG.warn("Access to non-public [{}] is blocked!", member);
return false;
}
- if (!checkStaticFieldAccess(member, memberModifiers)) {
+ if (!checkStaticFieldAccess(member)) {
LOG.warn("Access to static field [{}] is blocked!", member);
return false;
}
- // it needs to be before calling #checkStaticMethodAccess()
- if (checkEnumAccess(target, member)) {
- LOG.trace("Allowing access to enum: target [{}], member [{}]", target, member);
- return true;
- }
-
- if (!checkStaticMethodAccess(member, memberModifiers)) {
+ if (!checkStaticMethodAccess(target, member)) {
LOG.warn("Access to static method [{}] is blocked!", member);
return false;
}
- if (isClassExcluded(memberClass)) {
- LOG.warn("Declaring class of member type [{}] is excluded!", member);
+ if (!checkDefaultPackageAccess(target, member)) {
return false;
}
- if (targetClass != memberClass && isClassExcluded(targetClass)) {
- LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
+ if (!checkExclusionList(target, member)) {
return false;
}
- if (disallowDefaultPackageAccess) {
- if (targetClass.getPackage() == null || targetClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!", targetClass);
- return false;
- }
- if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) {
- LOG.warn("Class [{}] from the default package is excluded!", memberClass);
- return false;
- }
+ if (!isAcceptableProperty(propertyName)) {
+ return false;
}
- if (isPackageExcluded(targetClass)) {
+ return true;
+ }
+
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkExclusionList(Object target, Member member) {
+ Class<?> memberClass = member.getClass();
+ if (isClassExcluded(memberClass)) {
+ LOG.warn("Declaring class of member type [{}] is excluded!", memberClass);
+ return false;
+ }
+ if (isPackageExcluded(memberClass)) {
LOG.warn("Package [{}] of target class [{}] of target [{}] is excluded!",
- targetClass.getPackage(),
- targetClass,
+ memberClass.getPackage(),
+ memberClass,
target);
return false;
}
+ if (target == null || target.getClass() == memberClass) {
+ return true;
+ }
+ Class<?> targetClass = target.getClass();
+ if (isClassExcluded(targetClass)) {
+ LOG.warn("Target class [{}] of target [{}] is excluded!", targetClass, target);
+ return false;
+ }
+ if (isPackageExcluded(targetClass)) {
+ LOG.warn("Package [{}] of member [{}] are excluded!", targetClass.getPackage(), member);
+ return false;
+ }
+ return true;
+ }
- if (targetClass != memberClass && isPackageExcluded(memberClass)) {
- LOG.warn("Package [{}] of member [{}] are excluded!", memberClass.getPackage(), member);
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkDefaultPackageAccess(Object target, Member member) {
+ if (!disallowDefaultPackageAccess) {
+ return true;
+ }
+ Class<?> memberClass = member.getClass();
+ if (memberClass.getPackage() == null || memberClass.getPackage().getName().isEmpty()) {
+ LOG.warn("Class [{}] from the default package is excluded!", memberClass);
+ return false;
+ }
+ if (target == null || target.getClass() == memberClass) {
+ return true;
+ }
+ Class<?> targetClass = target.getClass();
+ if (targetClass.getPackage() == null || targetClass.getPackage().getName().isEmpty()) {
+ LOG.warn("Class [{}] from the default package is excluded!", targetClass);
return false;
}
+ return true;
+ }
- return isAcceptableProperty(propertyName);
+ /**
+ * @return {@code true} if member access is allowed
+ */
+ protected boolean checkProxyMemberAccess(Object target, Member member) {
+ return !disallowProxyMemberAccess || target == null || !ProxyUtil.isProxyMember(member, target);
}
/**
* Check access for static method (via modifiers).
- *
+ * <p>
* Note: For non-static members, the result is always true.
*
- * @param member
- * @param memberModifiers
- *
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkStaticMethodAccess(Member member, int memberModifiers) {
- return !Modifier.isStatic(memberModifiers) || member instanceof Field;
+ protected boolean checkStaticMethodAccess(Object target, Member member) {
+ if (checkEnumAccess(target, member)) {
+ LOG.trace("Exempting enum from static method check: target [{}], member [{}]", target, member);
+ return true;
+ }
+ return member instanceof Field || !Modifier.isStatic(member.getModifiers());
}
/**
@@ -194,16 +224,13 @@ public class SecurityMemberAccess implements MemberAccess {
* <p>
* Note: For non-static members, the result is always true.
*
- * @param member
- * @param memberModifiers
- * @return
+ * @return {@code true} if member access is allowed
*/
- protected boolean checkStaticFieldAccess(Member member, int memberModifiers) {
- if (Modifier.isStatic(memberModifiers) && member instanceof Field) {
- return allowStaticFieldAccess;
- } else {
+ protected boolean checkStaticFieldAccess(Member member) {
+ if (allowStaticFieldAccess) {
return true;
}
+ return !(member instanceof Field && Modifier.isStatic(member.getModifiers()));
}
/**
@@ -218,6 +245,9 @@ public class SecurityMemberAccess implements MemberAccess {
return Modifier.isPublic(memberModifiers);
}
+ /**
+ * @return {@code true} if member access is allowed
+ */
protected boolean checkEnumAccess(Object target, Member member) {
if (target instanceof Class) {
final Class<?> clazz = (Class<?>) target;
@@ -227,7 +257,8 @@ public class SecurityMemberAccess implements MemberAccess {
}
protected boolean isPackageExcluded(Class<?> clazz) {
- return !excludedPackageExemptClasses.contains(clazz.getName()) && (isExcludedPackageNames(clazz) || isExcludedPackageNamePatterns(clazz));
+ return !excludedPackageExemptClasses.contains(clazz.getName()) && (isExcludedPackageNames(clazz) || isExcludedPackageNamePatterns(
+ clazz));
}
public static String toPackageName(Class<?> clazz) {
@@ -260,8 +291,11 @@ public class SecurityMemberAccess implements MemberAccess {
return excludedClasses.contains(clazz.getName());
}
+ /**
+ * @return {@code true} if member access is allowed
+ */
protected boolean isAcceptableProperty(String name) {
- return name == null || ((!isExcluded(name)) && isAccepted(name));
+ return name == null || !isExcluded(name) && isAccepted(name);
}
protected boolean isAccepted(String paramName) {