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 <g...@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) {
