WW-5525 Fixes NPE when checking if expressions is acceptable

apache · Feb 1, 2025 · a8bce0f · a8bce0f
1 parent f98b675
commit a8bce0f
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 10 deletions.
diff --git a/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java b/core/src/main/java/org/apache/struts2/ognl/SecurityMemberAccess.java
@@ -141,7 +141,7 @@ public void restore(Map context, Object target, Member member, String propertyNa
     public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
         LOG.debug("Checking access for [target: {}, member: {}, property: {}]", target, member, propertyName);
 
-        if (target != null) {
+        if (target != null && member != null) {
             // Special case: Target is a Class object but not Class.class
             if (Class.class.equals(target.getClass()) && !Class.class.equals(target)) {
                 if (!isStatic(member) && !Constructor.class.equals(member.getClass())) {
@@ -157,44 +157,44 @@ public boolean isAccessible(Map context, Object target, Member member, String pr
             }
         }
 
-        if (!checkProxyObjectAccess(target)) {
+        if (target != null && !checkProxyObjectAccess(target)) {
             LOG.warn("Access to proxy is blocked! Target [{}], proxy class [{}]", target, target.getClass().getName());
             return false;
         }
 
-        if (!checkProxyMemberAccess(target, member)) {
+        if (target != null && member != null && !checkProxyMemberAccess(target, member)) {
             LOG.warn("Access to proxy is blocked! Member class [{}] of target [{}], member [{}]", member.getDeclaringClass(), target, member);
             return false;
         }
 
-        if (!checkPublicMemberAccess(member)) {
+        if (member != null && !checkPublicMemberAccess(member)) {
             LOG.warn("Access to non-public [{}] is blocked!", member);
             return false;
         }
 
-        if (!checkStaticFieldAccess(member)) {
+        if (member != null && !checkStaticFieldAccess(member)) {
             LOG.warn("Access to static field [{}] is blocked!", member);
             return false;
         }
 
-        if (!checkStaticMethodAccess(member)) {
+        if (member != null && !checkStaticMethodAccess(member)) {
             LOG.warn("Access to static method [{}] is blocked!", member);
             return false;
         }
 
-        if (!checkDefaultPackageAccess(target, member)) {
+        if (target != null && member != null && !checkDefaultPackageAccess(target, member)) {
             return false;
         }
 
-        if (!checkExclusionList(target, member)) {
+        if (target != null && member != null && !checkExclusionList(target, member)) {
             return false;
         }
 
-        if (!checkAllowlist(target, member)) {
+        if (target != null && member != null && !checkAllowlist(target, member)) {
             return false;
         }
 
-        if (!isAcceptableProperty(propertyName)) {
+        if (propertyName != null && !isAcceptableProperty(propertyName)) {
             return false;
         }
 

diff --git a/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java b/core/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessTest.java
@@ -649,6 +649,7 @@ public void testBlockedStaticFieldWhenFlagIsFalse() throws Exception {
     @Test
     public void testBlockedStaticFieldWhenClassIsExcluded() throws Exception {
         // given
+        assignNewSma(false);
         sma.useExcludedClasses(String.join(",", Class.class.getName(), StaticTester.class.getName()));
 
         // when

diff --git a/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java b/plugins/spring/src/test/java/org/apache/struts2/ognl/SecurityMemberAccessProxyTest.java
@@ -87,4 +87,80 @@ public void allowAllProxyAccess() {
         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());
+        assertFalse(sma.isAccessible(context, proxy.getAction(), null, ""));
+    }
+
+    @Test
+    public void nullMemberAndTargetAllowedAndMemberNotAllowed() {
+        sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
+        sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
+        assertTrue(sma.isAccessible(context, proxy.getAction(), null, ""));
+    }
+
+    @Test
+    public void nullMemberAndTargetNotAllowedAndMemberAllowed() {
+        sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
+        sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
+        assertFalse(sma.isAccessible(context, proxy.getAction(), null, ""));
+    }
+
+    @Test
+    public void nullTargetAndMemberAndTargetAndMemberNotAllowed() {
+        sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
+        sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
+        assertTrue(sma.isAccessible(context, null, null, ""));
+    }
+
+    @Test
+    public void nullTargetAndMemberAndTargetNotAllowedAndMemberAllowed() {
+        sma.useDisallowProxyObjectAccess(Boolean.TRUE.toString());
+        sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
+        assertTrue(sma.isAccessible(context, null, null, ""));
+    }
+
+    @Test
+    public void nullTargetAndMemberAndTargetAllowedAndMemberNotAllowed() {
+        sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
+        sma.useDisallowProxyMemberAccess(Boolean.TRUE.toString());
+        assertTrue(sma.isAccessible(context, null, null, ""));
+    }
+
+    @Test
+    public void nullTargetAndMemberAndTargetAndMemberAllowed() {
+        sma.useDisallowProxyObjectAccess(Boolean.FALSE.toString());
+        sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
+        assertTrue(sma.isAccessible(context, null, null, ""));
+    }
+
+    @Test
+    public void nullPropertyName() {
+        sma.useDisallowProxyMemberAccess(Boolean.FALSE.toString());
+        assertTrue(sma.isAccessible(context, proxy.getAction(), proxyObjectProxyMember, null));
+    }
 }