Apache/struts
1
0
Fork 0
mirror of https://github.com/apache/struts.git synced 2026-08-06 15:17:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1220 from apache/WW-5525-proxyutil-npe-67

Browse Source 
6.7: WW-5525 Fix NPE in ProxyUtil for SecurityMemberAccess originating static members
This commit is contained in:
Kusal Kithul-Godage
2025-02-17 19:11:21 +11:00
committed by GitHub
parent 9b044377ac d35ec15c76
commit ef90b3ba3c
5 changed files with 124 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
+4 -1
View File
@@ -160,6 +160,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)) {
@@ -228,7 +231,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
core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java
+2 -1
View File
@@ -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;
}
core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
+28
View File
@@ -1233,6 +1233,34 @@ public class OgnlValueStackTest extends XWorkTestCase {
assertNull("accessed private field (result not null) ?", accessedValue);
}
public void testFindValueWithConstructorAndProxyChecks() {
Map<String, String> properties = new HashMap<>();
properties.put(StrutsConstants.STRUTS_DISALLOW_PROXY_OBJECT_ACCESS, Boolean.TRUE.toString());
properties.put(StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, Boolean.TRUE.toString());
loadButSet(properties);
refreshContainerFields();
String value = "test";
String ognlResult = (String) vs.findValue(
"new com.opensymphony.xwork2.ognl.OgnlValueStackTest$ValueHolder('" + value + "').value", String.class);
assertEquals(value, ognlResult);
}
@SuppressWarnings({"unused"})
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;
plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java
+88
View File
@@ -31,6 +31,7 @@ import java.util.HashMap;
import java.util.Map;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
public class SecurityMemberAccessProxyTest extends XWorkJUnit4TestCase {
@@ -87,4 +88,91 @@ public class SecurityMemberAccessProxyTest extends XWorkJUnit4TestCase {
assertTrue(sma.isAccessible(context, proxy.getAction(), proxyObjectProxyMember, ""));
assertTrue(sma.isAccessible(context, proxy.getAction(), proxyObjectNonProxyMember, ""));
}
@Test
public void nullTargetAndTargetAndMemberNotAllowed() {
sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
assertTrue(sma.isAccessible(context, null, proxyObjectProxyMember, ""));
}
@Test
public void nullTargetAndTargetAllowedAndMemberNotAllowed() {
sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
assertTrue(sma.isAccessible(context, null, proxyObjectProxyMember, ""));
}
@Test
public void nullTargetAndTargetAndMemberAllowed() {
sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
assertTrue(sma.isAccessible(context, null, proxyObjectProxyMember, ""));
}
@Test
public void nullMemberAndTargetAndMemberNotAllowed() {
sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
Object action = proxy.getAction();
assertThrows("Member cannot be null!", IllegalArgumentException.class,
() -> sma.isAccessible(context, action, null, ""));
}
@Test
public void nullMemberAndTargetAllowedAndMemberNotAllowed() {
sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
Object action = proxy.getAction();
assertThrows("Member cannot be null!", IllegalArgumentException.class,
() -> sma.isAccessible(context, action, null, ""));
}
@Test
public void nullMemberAndTargetNotAllowedAndMemberAllowed() {
sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
Object action = proxy.getAction();
assertThrows("Member cannot be null!", IllegalArgumentException.class,
() -> sma.isAccessible(context, action, null, ""));
}
@Test
public void nullTargetAndMemberAndTargetAndMemberNotAllowed() {
sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
assertThrows("Member cannot be null!", IllegalArgumentException.class,
() -> sma.isAccessible(context, null, null, ""));
}
@Test
public void nullTargetAndMemberAndTargetNotAllowedAndMemberAllowed() {
sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
assertThrows("Member cannot be null!", IllegalArgumentException.class,
() -> sma.isAccessible(context, null, null, ""));
}
@Test
public void nullTargetAndMemberAndTargetAllowedAndMemberNotAllowed() {
sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
assertThrows("Member cannot be null!", IllegalArgumentException.class,
() -> sma.isAccessible(context, null, null, ""));
}
@Test
public void nullTargetAndMemberAndTargetAndMemberAllowed() {
sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
assertThrows("Member cannot be null!", IllegalArgumentException.class,
() -> sma.isAccessible(context, null, null, ""));
}
@Test
public void nullPropertyName() {
sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
Object action = proxy.getAction();
assertTrue(sma.isAccessible(context, action, proxyObjectProxyMember, null));
}
}
plugins/spring/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java
+2
View File
@@ -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));