diff --git a/core/pom.xml b/core/pom.xml index 3fcfaaf7b..8165d191a 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -230,6 +230,15 @@ org.apache.commons commons-text + + + + org.hibernate + hibernate-core + 5.6.15.Final + true + + org.springframework spring-test diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java index 29dc7fc7f..e503f4998 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java @@ -32,6 +32,7 @@ public class DefaultOgnlCacheFactory implements OgnlCacheFactory implements OgnlCacheFactory buildOgnlCache() { - return buildOgnlCache(getCacheMaxSize(), DEFAULT_INIT_CAPACITY, DEFAULT_LOAD_FACTOR, defaultCacheType); + return buildOgnlCache(getCacheMaxSize(), initialCapacity, DEFAULT_LOAD_FACTOR, defaultCacheType); } @Override 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 510a65c60..b0ee1f21c 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -87,6 +87,7 @@ public class SecurityMemberAccess implements MemberAccess { private boolean enforceAllowlistEnabled = false; private Set> allowlistClasses = emptySet(); private Set allowlistPackageNames = emptySet(); + private boolean disallowProxyObjectAccess = false; private boolean disallowProxyMemberAccess = false; private boolean disallowDefaultPackageAccess = false; @@ -160,6 +161,11 @@ public class SecurityMemberAccess implements MemberAccess { } } + if (!checkProxyObjectAccess(target)) { + LOG.warn("Access to proxy is blocked! Target [{}], proxy class [{}]", target, target.getClass().getName()); + return false; + } + if (!checkProxyMemberAccess(target, member)) { LOG.warn("Access to proxy is blocked! Member class [{}] of target [{}], member [{}]", member.getDeclaringClass(), target, member); return false; @@ -286,7 +292,14 @@ public class SecurityMemberAccess implements MemberAccess { } /** - * @return {@code true} if member access is allowed + * @return {@code true} if proxy object access is allowed + */ + protected boolean checkProxyObjectAccess(Object target) { + return !(disallowProxyObjectAccess && ProxyUtil.isProxy(target)); + } + + /** + * @return {@code true} if proxy member access is allowed */ protected boolean checkProxyMemberAccess(Object target, Member member) { return !(disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)); @@ -448,6 +461,11 @@ public class SecurityMemberAccess implements MemberAccess { this.allowlistPackageNames = toPackageNamesSet(commaDelimitedPackageNames); } + @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, required = false) + public void useDisallowProxyObjectAccess(String disallowProxyObjectAccess) { + this.disallowProxyObjectAccess = BooleanUtils.toBoolean(disallowProxyObjectAccess); + } + @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, required = false) public void useDisallowProxyMemberAccess(String disallowProxyMemberAccess) { this.disallowProxyMemberAccess = BooleanUtils.toBoolean(disallowProxyMemberAccess); 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 9b0e7d4ea..aa8d23d76 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java @@ -18,13 +18,20 @@ */ package com.opensymphony.xwork2.util; +import com.opensymphony.xwork2.ognl.DefaultOgnlCacheFactory; +import com.opensymphony.xwork2.ognl.OgnlCache; +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.proxy.HibernateProxy; -import java.lang.reflect.*; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Member; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.Proxy; /** * ProxyUtil @@ -38,11 +45,13 @@ public class ProxyUtil { private static final String SPRING_SPRINGPROXY_CLASS_NAME = "org.springframework.aop.SpringProxy"; private static final String SPRING_SINGLETONTARGETSOURCE_CLASS_NAME = "org.springframework.aop.target.SingletonTargetSource"; private static final String SPRING_TARGETCLASSAWARE_CLASS_NAME = "org.springframework.aop.TargetClassAware"; - - private static final Map, Boolean> isProxyCache = - new ConcurrentHashMap<>(256); - private static final Map isProxyMemberCache = - new ConcurrentHashMap<>(256); + private static final String HIBERNATE_HIBERNATEPROXY_CLASS_NAME = "org.hibernate.proxy.HibernateProxy"; + private static final int CACHE_MAX_SIZE = 10000; + private static final int CACHE_INITIAL_CAPACITY = 256; + private static final OgnlCache, Boolean> isProxyCache = new DefaultOgnlCacheFactory, Boolean>( + CACHE_MAX_SIZE, OgnlCacheFactory.CacheType.WTLFU, CACHE_INITIAL_CAPACITY).buildOgnlCache(); + private static final OgnlCache isProxyMemberCache = new DefaultOgnlCacheFactory( + CACHE_MAX_SIZE, OgnlCacheFactory.CacheType.WTLFU, CACHE_INITIAL_CAPACITY).buildOgnlCache(); /** * Determine the ultimate target class of the given instance, traversing @@ -75,7 +84,7 @@ public class ProxyUtil { return flag; } - boolean isProxy = isSpringAopProxy(object); + boolean isProxy = isSpringAopProxy(object) || isHibernateProxy(object); isProxyCache.put(clazz, isProxy); return isProxy; @@ -87,7 +96,7 @@ public class ProxyUtil { * @param object the object to check */ public static boolean isProxyMember(Member member, Object object) { - if (!Modifier.isStatic(member.getModifiers()) && !isProxy(object)) { + if (!Modifier.isStatic(member.getModifiers()) && !isProxy(object) && !isHibernateProxy(object)) { return false; } @@ -96,12 +105,41 @@ public class ProxyUtil { return flag; } - boolean isProxyMember = isSpringProxyMember(member); + boolean isProxyMember = isSpringProxyMember(member) || isHibernateProxyMember(member); isProxyMemberCache.put(member, isProxyMember); return isProxyMember; } + /** + * Check whether the given object is a Hibernate proxy. + * + * @param object the object to check + */ + public static boolean isHibernateProxy(Object object) { + try { + return HibernateProxy.class.isAssignableFrom(object.getClass()); + } catch (NoClassDefFoundError ignored) { + return false; + } + } + + /** + * Check whether the given member is a member of a Hibernate proxy. + * + * @param member the member to check + */ + public static boolean isHibernateProxyMember(Member member) { + try { + Class clazz = ClassLoaderUtil.loadClass(HIBERNATE_HIBERNATEPROXY_CLASS_NAME, ProxyUtil.class); + if (hasMember(clazz, member)) + return true; + } catch (ClassNotFoundException ignored) { + } + + return false; + } + /** * Determine the ultimate target class of the given spring bean instance, traversing * not only a top-level spring proxy but any number of nested spring proxies as well — diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 3d0d1a00d..10d9fa5b7 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -480,6 +480,7 @@ public final class StrutsConstants { public static final String STRUTS_TEXT_PROVIDER_FACTORY = "struts.textProviderFactory"; public static final String STRUTS_LOCALIZED_TEXT_PROVIDER = "struts.localizedTextProvider"; + public static final String STRUTS_DISALLOW_PROXY_OBJECT_ACCESS = "struts.disallowProxyObjectAccess"; public static final String STRUTS_DISALLOW_PROXY_MEMBER_ACCESS = "struts.disallowProxyMemberAccess"; public static final String STRUTS_DISALLOW_DEFAULT_PACKAGE_ACCESS = "struts.disallowDefaultPackageAccess"; diff --git a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java index 2b854243d..6106aad37 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java @@ -145,6 +145,7 @@ public class ConstantConfig { private String strictMethodInvocationMethodRegex; private BeanConfig textProviderFactory; private BeanConfig localizedTextProvider; + private Boolean disallowProxyObjectAccess; private Boolean disallowProxyMemberAccess; private Integer ognlAutoGrowthCollectionLimit; private String staticContentPath; @@ -279,6 +280,7 @@ public class ConstantConfig { map.put(StrutsConstants.STRUTS_SMI_METHOD_REGEX, strictMethodInvocationMethodRegex); map.put(StrutsConstants.STRUTS_TEXT_PROVIDER_FACTORY, beanConfToString(textProviderFactory)); map.put(StrutsConstants.STRUTS_LOCALIZED_TEXT_PROVIDER, beanConfToString(localizedTextProvider)); + map.put(StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, Objects.toString(disallowProxyObjectAccess, null)); map.put(StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, Objects.toString(disallowProxyMemberAccess, null)); map.put(StrutsConstants.STRUTS_OGNL_AUTO_GROWTH_COLLECTION_LIMIT, Objects.toString(ognlAutoGrowthCollectionLimit, null)); map.put(StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH, Objects.toString(staticContentPath, StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH)); @@ -1360,6 +1362,14 @@ public class ConstantConfig { this.localizedTextProvider = new BeanConfig(clazz, clazz.getName()); } + public Boolean getDisallowProxyObjectAccess() { + return disallowProxyObjectAccess; + } + + public void setDisallowProxyObjectAccess(Boolean disallowProxyObjectAccess) { + this.disallowProxyObjectAccess = disallowProxyObjectAccess; + } + public Boolean getDisallowProxyMemberAccess() { return disallowProxyMemberAccess; } diff --git a/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java b/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java index 4d8046de9..3838ca9ae 100644 --- a/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java +++ b/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java @@ -29,6 +29,11 @@ import java.util.Map; public class SecurityMemberAccessProxyTest extends XWorkTestCase { private Map context; + private ActionProxy proxy; + private Map members; + private final SecurityMemberAccess sma = new SecurityMemberAccess(true); + private final String PROXY_MEMBER_METHOD = "isExposeProxy"; + private final String TEST_SUB_BEAN_CLASS_METHOD = "setIssueId"; @Override public void setUp() throws Exception { @@ -39,30 +44,51 @@ public class SecurityMemberAccessProxyTest extends XWorkTestCase { XmlConfigurationProvider provider = new StrutsXmlConfigurationProvider("com/opensymphony/xwork2/spring/actionContext-xwork.xml"); container.inject(provider); loadConfigurationProviders(provider); + + // Setup proxy object + setupProxy(); } public void testProxyAccessIsBlocked() throws Exception { - ActionProxy proxy = actionProxyFactory.createActionProxy(null, - "chaintoAOPedTestSubBeanAction", null, context); + members.values().forEach(member -> { + // When disallowProxyObjectAccess is set to true, and disallowProxyMemberAccess is set to false, the proxy access is blocked + sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString()); + sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString()); + assertFalse(sma.isAccessible(context, proxy.getAction(), member, "")); - SecurityMemberAccess sma = new SecurityMemberAccess(true); + // When disallowProxyObjectAccess is set to true, and disallowProxyMemberAccess is set to true, the proxy access is blocked + sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString()); + sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString()); + assertFalse(sma.isAccessible(context, proxy.getAction(), member, "")); + }); + + // When disallowProxyObjectAccess is set to false, and disallowProxyMemberAccess is set to true, the proxy member access is blocked + sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString()); sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString()); - - Member member = proxy.getAction().getClass().getMethod("isExposeProxy"); - - boolean accessible = sma.isAccessible(context, proxy.getAction(), member, ""); - assertFalse(accessible); + assertFalse(sma.isAccessible(context, proxy.getAction(), members.get(PROXY_MEMBER_METHOD), "")); } public void testProxyAccessIsAccessible() throws Exception { - ActionProxy proxy = actionProxyFactory.createActionProxy(null, - "chaintoAOPedTestSubBeanAction", null, context); + members.values().forEach(member -> { + // When disallowProxyObjectAccess is set to false, and disallowProxyMemberAccess is set to false, the proxy access is allowed + sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString()); + sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString()); + assertTrue(sma.isAccessible(context, proxy.getAction(), member, "")); + }); - SecurityMemberAccess sma = new SecurityMemberAccess(true); + // When disallowProxyObjectAccess is set to false, and disallowProxyMemberAccess is set to true, the original class member access is allowed + sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString()); + sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString()); + assertTrue(sma.isAccessible(context, proxy.getAction(), members.get(TEST_SUB_BEAN_CLASS_METHOD), "")); + } - Member member = proxy.getAction().getClass().getMethod("isExposeProxy"); + private void setupProxy() throws NoSuchMethodException { + proxy = actionProxyFactory.createActionProxy(null, "chaintoAOPedTestSubBeanAction", null, context); - boolean accessible = sma.isAccessible(context, proxy.getAction(), member, ""); - assertTrue(accessible); + members = new HashMap<>(); + // method is proxy member + members.put(PROXY_MEMBER_METHOD, proxy.getAction().getClass().getMethod(PROXY_MEMBER_METHOD)); + // method is not proxy member but from POJO class + members.put(TEST_SUB_BEAN_CLASS_METHOD, proxy.getAction().getClass().getMethod(TEST_SUB_BEAN_CLASS_METHOD, String.class)); } }