From 2f814186c8f3d7beb8200285c94db67a7ac50edd Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 17 Jun 2024 21:02:49 +1000 Subject: [PATCH] WW-5428 Allowlist capability should resolve Hibernate proxies when disableProxyObjects is not set --- .../xwork2/ognl/SecurityMemberAccess.java | 12 +++++++ .../opensymphony/xwork2/util/ProxyUtil.java | 33 +++++++++++++++++++ 2 files changed, 45 insertions(+) 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 f882b2c58..fc74f9a14 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -209,6 +209,18 @@ public class SecurityMemberAccess implements MemberAccess { * @return {@code true} if member access is allowed */ protected boolean checkAllowlist(Object target, Member member) { + if (!disallowProxyObjectAccess && target != null && ProxyUtil.isProxy(target)) { + // If `disallowProxyObjectAccess` is not set, allow resolving Hibernate entities to their underlying + // classes/members. This allows the allowlist capability to continue working and offer some level of + // protection in applications where the developer has accepted the risk of allowing OGNL access to Hibernate + // entities. This is preferred to having to disable the allowlist capability entirely. + Object newTarget = ProxyUtil.getHibernateProxyTarget(target); + if (newTarget != target) { + target = newTarget; + member = ProxyUtil.resolveTargetMember(member, newTarget); + } + } + Class memberClass = member.getDeclaringClass(); if (!enforceAllowlistEnabled) { return true; diff --git a/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java b/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java index c169af20b..895cfb7ee 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java @@ -24,6 +24,7 @@ import com.opensymphony.xwork2.ognl.OgnlCacheFactory; import org.apache.commons.lang3.reflect.ConstructorUtils; import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.commons.lang3.reflect.MethodUtils; +import org.hibernate.Hibernate; import org.hibernate.proxy.HibernateProxy; import java.lang.reflect.Constructor; @@ -33,6 +34,8 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.lang.reflect.Proxy; +import static java.lang.reflect.Modifier.isPublic; + /** * ProxyUtil *

@@ -255,4 +258,34 @@ public class ProxyUtil { return false; } + + /** + * @return the target instance of the given object if it is a Hibernate proxy object, otherwise the given object + */ + public static Object getHibernateProxyTarget(Object object) { + try { + return Hibernate.unproxy(object); + } catch (NoClassDefFoundError ignored) { + return object; + } + } + + /** + * @return matching member on target object if one exists, otherwise the same member + */ + public static Member resolveTargetMember(Member proxyMember, Object target) { + int mod = proxyMember.getModifiers(); + if (proxyMember instanceof Method) { + if (isPublic(mod)) { + return MethodUtils.getMatchingAccessibleMethod(target.getClass(), proxyMember.getName(), ((Method) proxyMember).getParameterTypes()); + } else { + return MethodUtils.getMatchingMethod(target.getClass(), proxyMember.getName(), ((Method) proxyMember).getParameterTypes()); + } + } else if (proxyMember instanceof Field) { + return FieldUtils.getField(target.getClass(), proxyMember.getName(), isPublic(mod)); + } else if (proxyMember instanceof Constructor && isPublic(mod)) { + return ConstructorUtils.getMatchingAccessibleConstructor(target.getClass(), ((Constructor) proxyMember).getParameterTypes()); + } + return proxyMember; + } }