WW-5407 extend SecurityMemberAccess proxy detection to other proxies

This commit is contained in:
Jeffery Xie
2024-04-09 14:36:41 +10:00
parent c6d13f1016
commit 0074b70286
7 changed files with 135 additions and 27 deletions
+9
View File
@@ -230,6 +230,15 @@
<groupId>org.apache.commons</groupId>
<artifactId>commons-text</artifactId>
</dependency>
<!-- Optional used in com.opensymphony.xwork2.util.ProxyUtil to detect if object is HibernateProxy -->
<dependency>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-core</artifactId>
<version>5.6.15.Final</version>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-test</artifactId>
@@ -32,6 +32,7 @@ public class DefaultOgnlCacheFactory<Key, Value> implements OgnlCacheFactory<Key
private CacheType defaultCacheType;
private int cacheMaxSize;
private final int initialCapacity;
/**
* @deprecated since 6.4.0, use {@link #DefaultOgnlCacheFactory(int, CacheType)}
@@ -42,13 +43,18 @@ public class DefaultOgnlCacheFactory<Key, Value> implements OgnlCacheFactory<Key
}
public DefaultOgnlCacheFactory(int cacheMaxSize, CacheType defaultCacheType) {
this(cacheMaxSize, defaultCacheType, DEFAULT_INIT_CAPACITY);
}
public DefaultOgnlCacheFactory(int cacheMaxSize, CacheType defaultCacheType, int initialCapacity) {
this.cacheMaxSize = cacheMaxSize;
this.defaultCacheType = defaultCacheType;
this.initialCapacity = initialCapacity;
}
@Override
public OgnlCache<Key, Value> buildOgnlCache() {
return buildOgnlCache(getCacheMaxSize(), DEFAULT_INIT_CAPACITY, DEFAULT_LOAD_FACTOR, defaultCacheType);
return buildOgnlCache(getCacheMaxSize(), initialCapacity, DEFAULT_LOAD_FACTOR, defaultCacheType);
}
@Override
@@ -87,6 +87,7 @@ public class SecurityMemberAccess implements MemberAccess {
private boolean enforceAllowlistEnabled = false;
private Set<Class<?>> allowlistClasses = emptySet();
private Set<String> 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);
@@ -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;
/**
* <code>ProxyUtil</code>
@@ -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<Class<?>, Boolean> isProxyCache =
new ConcurrentHashMap<>(256);
private static final Map<Member, Boolean> 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<Class<?>, Boolean> isProxyCache = new DefaultOgnlCacheFactory<Class<?>, Boolean>(
CACHE_MAX_SIZE, OgnlCacheFactory.CacheType.WTLFU, CACHE_INITIAL_CAPACITY).buildOgnlCache();
private static final OgnlCache<Member, Boolean> isProxyMemberCache = new DefaultOgnlCacheFactory<Member, Boolean>(
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 &mdash;
@@ -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";
@@ -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;
}
@@ -29,6 +29,11 @@ import java.util.Map;
public class SecurityMemberAccessProxyTest extends XWorkTestCase {
private Map<String, Object> context;
private ActionProxy proxy;
private Map<String, Member> 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));
}
}