mirror of
https://github.com/apache/struts.git
synced 2026-08-05 14:47:09 +00:00
Merge pull request #1214 from apache/WW-5525-proxyutil-npe
WW-5525 Fix NPE in ProxyUtil for SecurityMemberAccess originating static members
This commit is contained in:
@@ -18,13 +18,13 @@
|
||||
*/
|
||||
package org.apache.struts2.ognl;
|
||||
|
||||
import org.apache.struts2.inject.Inject;
|
||||
import org.apache.struts2.util.ProxyUtil;
|
||||
import ognl.MemberAccess;
|
||||
import org.apache.commons.lang3.BooleanUtils;
|
||||
import org.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.Logger;
|
||||
import org.apache.struts2.StrutsConstants;
|
||||
import org.apache.struts2.inject.Inject;
|
||||
import org.apache.struts2.util.ProxyUtil;
|
||||
|
||||
import java.lang.reflect.AccessibleObject;
|
||||
import java.lang.reflect.Constructor;
|
||||
@@ -38,6 +38,10 @@ import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
import java.util.stream.IntStream;
|
||||
|
||||
import static java.text.MessageFormat.format;
|
||||
import static java.util.Collections.emptySet;
|
||||
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_CLASSES;
|
||||
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES;
|
||||
import static org.apache.struts2.util.ConfigParseUtil.toClassObjectsSet;
|
||||
import static org.apache.struts2.util.ConfigParseUtil.toClassesSet;
|
||||
import static org.apache.struts2.util.ConfigParseUtil.toNewClassesSet;
|
||||
@@ -45,10 +49,6 @@ import static org.apache.struts2.util.ConfigParseUtil.toNewPackageNamesSet;
|
||||
import static org.apache.struts2.util.ConfigParseUtil.toNewPatternsSet;
|
||||
import static org.apache.struts2.util.ConfigParseUtil.toPackageNamesSet;
|
||||
import static org.apache.struts2.util.DebugUtils.logWarningForFirstOccurrence;
|
||||
import static java.text.MessageFormat.format;
|
||||
import static java.util.Collections.emptySet;
|
||||
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_CLASSES;
|
||||
import static org.apache.struts2.StrutsConstants.STRUTS_ALLOWLIST_PACKAGE_NAMES;
|
||||
|
||||
/**
|
||||
* Allows access decisions to be made on the basis of whether a member is static or not.
|
||||
@@ -141,6 +141,9 @@ 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);
|
||||
|
||||
if (member == null) {
|
||||
throw new IllegalArgumentException("Member cannot be null!");
|
||||
}
|
||||
if (target != null) {
|
||||
// Special case: Target is a Class object but not Class.class
|
||||
if (Class.class.equals(target.getClass()) && !Class.class.equals(target)) {
|
||||
@@ -209,7 +212,7 @@ public class SecurityMemberAccess implements MemberAccess {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!disallowProxyObjectAccess && target != null && ProxyUtil.isProxy(target)) {
|
||||
if (!disallowProxyObjectAccess && 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
|
||||
|
||||
@@ -18,12 +18,12 @@
|
||||
*/
|
||||
package org.apache.struts2.util;
|
||||
|
||||
import org.apache.struts2.ognl.DefaultOgnlCacheFactory;
|
||||
import org.apache.struts2.ognl.OgnlCache;
|
||||
import org.apache.struts2.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.apache.struts2.ognl.DefaultOgnlCacheFactory;
|
||||
import org.apache.struts2.ognl.OgnlCache;
|
||||
import org.apache.struts2.ognl.OgnlCacheFactory;
|
||||
import org.hibernate.Hibernate;
|
||||
import org.hibernate.proxy.HibernateProxy;
|
||||
|
||||
@@ -81,6 +81,7 @@ public class ProxyUtil {
|
||||
* @param object the object to check
|
||||
*/
|
||||
public static boolean isProxy(Object object) {
|
||||
if (object == null) return false;
|
||||
Class<?> clazz = object.getClass();
|
||||
Boolean flag = isProxyCache.get(clazz);
|
||||
if (flag != null) {
|
||||
@@ -121,7 +122,7 @@ public class ProxyUtil {
|
||||
*/
|
||||
public static boolean isHibernateProxy(Object object) {
|
||||
try {
|
||||
return HibernateProxy.class.isAssignableFrom(object.getClass());
|
||||
return object != null && HibernateProxy.class.isAssignableFrom(object.getClass());
|
||||
} catch (NoClassDefFoundError ignored) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -18,17 +18,26 @@
|
||||
*/
|
||||
package org.apache.struts2.ognl;
|
||||
|
||||
import ognl.OgnlException;
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
import org.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.core.LogEvent;
|
||||
import org.apache.logging.log4j.core.Logger;
|
||||
import org.apache.logging.log4j.core.appender.AbstractAppender;
|
||||
import org.apache.struts2.SimpleAction;
|
||||
import org.apache.struts2.StrutsConstants;
|
||||
import org.apache.struts2.StrutsException;
|
||||
import org.apache.struts2.TestBean;
|
||||
import org.apache.struts2.text.TextProvider;
|
||||
import org.apache.struts2.XWorkTestCase;
|
||||
import org.apache.struts2.config.ConfigurationException;
|
||||
import org.apache.struts2.config.DefaultPropertiesProvider;
|
||||
import org.apache.struts2.conversion.impl.ConversionData;
|
||||
import org.apache.struts2.conversion.impl.XWorkConverter;
|
||||
import org.apache.struts2.inject.ContainerBuilder;
|
||||
import org.apache.struts2.ognl.accessor.RootAccessor;
|
||||
import org.apache.struts2.test.StubConfigurationProvider;
|
||||
import org.apache.struts2.test.TestBean2;
|
||||
import org.apache.struts2.text.TextProvider;
|
||||
import org.apache.struts2.util.Bar;
|
||||
import org.apache.struts2.util.BarJunior;
|
||||
import org.apache.struts2.util.Cat;
|
||||
@@ -37,15 +46,6 @@ import org.apache.struts2.util.Foo;
|
||||
import org.apache.struts2.util.ValueStackFactory;
|
||||
import org.apache.struts2.util.location.LocatableProperties;
|
||||
import org.apache.struts2.util.reflection.ReflectionContextState;
|
||||
import ognl.OgnlException;
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
import org.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.core.LogEvent;
|
||||
import org.apache.logging.log4j.core.Logger;
|
||||
import org.apache.logging.log4j.core.appender.AbstractAppender;
|
||||
import org.apache.struts2.StrutsConstants;
|
||||
import org.apache.struts2.StrutsException;
|
||||
import org.apache.struts2.config.DefaultPropertiesProvider;
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.io.ByteArrayOutputStream;
|
||||
@@ -1234,6 +1234,33 @@ public class OgnlValueStackTest extends XWorkTestCase {
|
||||
assertNull("accessed private field (result not null) ?", accessedValue);
|
||||
}
|
||||
|
||||
public void testFindValueWithConstructorAndProxyChecks() {
|
||||
loadButSet(Map.of(
|
||||
StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, Boolean.TRUE.toString(),
|
||||
StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, Boolean.TRUE.toString()));
|
||||
refreshContainerFields();
|
||||
|
||||
String value = "test";
|
||||
String ognlResult = (String) vs.findValue(
|
||||
"new org.apache.struts2.ognl.OgnlValueStackTest$ValueHolder('" + value + "').value", String.class);
|
||||
|
||||
assertEquals(value, ognlResult);
|
||||
}
|
||||
|
||||
@SuppressWarnings({"unused", "ClassCanBeRecord"})
|
||||
public static class ValueHolder {
|
||||
// See testFindValueWithConstructorAndProxyChecks
|
||||
private final String value;
|
||||
|
||||
public ValueHolder(String value) {
|
||||
this.value = value;
|
||||
}
|
||||
|
||||
public String getValue() {
|
||||
return value;
|
||||
}
|
||||
}
|
||||
|
||||
static class BadJavaBean {
|
||||
private int count;
|
||||
private int count2;
|
||||
|
||||
@@ -23,9 +23,9 @@ import org.apache.struts2.SimpleAction;
|
||||
import org.apache.struts2.TestBean;
|
||||
import org.apache.struts2.TestSubBean;
|
||||
import org.apache.struts2.XWorkTestCase;
|
||||
import org.apache.struts2.config.StrutsXmlConfigurationProvider;
|
||||
import org.apache.struts2.config.providers.XmlConfigurationProvider;
|
||||
import org.apache.struts2.util.ProxyUtil;
|
||||
import org.apache.struts2.config.StrutsXmlConfigurationProvider;
|
||||
import org.springframework.context.ApplicationContext;
|
||||
|
||||
/**
|
||||
@@ -46,6 +46,8 @@ public class SpringProxyUtilTest extends XWorkTestCase {
|
||||
}
|
||||
|
||||
public void testIsProxy() throws Exception {
|
||||
assertFalse(ProxyUtil.isProxy(null));
|
||||
|
||||
Object simpleAction = appContext.getBean("simple-action");
|
||||
assertFalse(ProxyUtil.isProxy(simpleAction));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user