(struts) 01/02: WW-5525 Fix NPE in ProxyUtil for SecurityMemberAccess originating static members

[kusal]

This is an automated email from the ASF dual-hosted git repository.

kusal pushed a commit to branch WW-5525-proxyutil-npe-67
in repository https://gitbox.apache.org/repos/asf/struts.git

commit 14254e602bb17c64af7e92576c777842feefce3b
Author: Kusal Kithul-Godage <g...@kusal.io>
AuthorDate: Thu Feb 6 12:02:40 2025 +1100

    WW-5525 Fix NPE in ProxyUtil for SecurityMemberAccess originating static 
members
---
 .../xwork2/ognl/SecurityMemberAccess.java          |  5 +++-
 .../com/opensymphony/xwork2/util/ProxyUtil.java    |  3 ++-
 .../xwork2/ognl/OgnlValueStackTest.java            | 28 ++++++++++++++++++++++
 .../xwork2/spring/SpringProxyUtilTest.java         |  2 ++
 4 files changed, 36 insertions(+), 2 deletions(-)

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 f15b50af1..3963ac730 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
@@ -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
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 895cfb7ee..22c344446 100644
--- a/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java
+++ b/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java
@@ -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;
         }
diff --git 
a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java 
b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
index 7fb560c5b..86a83cd8c 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
@@ -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 org.apache.struts2.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;
diff --git 
a/plugins/spring/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java
 
b/plugins/spring/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java
index 7b77294e5..dc6e58cb3 100644
--- 
a/plugins/spring/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java
+++ 
b/plugins/spring/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java
@@ -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));