(struts) 01/02: WW-5418 Forbid enums

[kusal]

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

kusal pushed a commit to branch WW-5418-struts-sec
in repository https://gitbox.apache.org/repos/asf/struts.git

commit 100f5052d40a4bfbc128661ea489b0c8568a78a0
Author: Kusal Kithul-Godage <g...@kusal.io>
AuthorDate: Thu Apr 18 11:13:56 2024 +1000

    WW-5418 Forbid enums
---
 .../opensymphony/xwork2/ognl/SecurityMemberAccess.java   | 16 ----------------
 .../com/opensymphony/xwork2/ognl/OgnlValueStackTest.java |  8 ++++----
 .../xwork2/ognl/SecurityMemberAccessTest.java            |  2 +-
 3 files changed, 5 insertions(+), 21 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 b0ee1f21c..43ae99240 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
@@ -31,7 +31,6 @@ import org.apache.struts2.ognl.ThreadAllowlist;
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Field;
 import java.lang.reflect.Member;
-import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -313,10 +312,6 @@ public class SecurityMemberAccess implements MemberAccess {
      * @return {@code true} if member access is allowed
      */
     protected boolean checkStaticMethodAccess(Member member) {
-        if (checkEnumAccess(member)) {
-            LOG.trace("Exempting Enum#values from static method check: class 
[{}]", member.getDeclaringClass());
-            return true;
-        }
         return member instanceof Field || !isStatic(member);
     }
 
@@ -347,17 +342,6 @@ public class SecurityMemberAccess implements MemberAccess {
         return Modifier.isPublic(member.getModifiers());
     }
 
-    /**
-     * @return {@code true} if member access is allowed
-     */
-    protected boolean checkEnumAccess(Member member) {
-        return member.getDeclaringClass().isEnum()
-                && isStatic(member)
-                && member instanceof Method
-                && member.getName().equals("values")
-                && ((Method) member).getParameterCount() == 0;
-    }
-
     protected boolean isPackageExcluded(Class<?> clazz) {
         return !excludedPackageExemptClasses.contains(clazz.getName()) && 
(isExcludedPackageNames(clazz) || isExcludedPackageNamePatterns(clazz));
     }
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 3bdfd67fc..7fb560c5b 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
@@ -437,12 +437,12 @@ public class OgnlValueStackTest extends XWorkTestCase {
     }
 
     /**
-     * Allow access Enums without enabling access to static methods
+     * Enum methods should also be banned alongside static methods
      */
     public void testEnum() throws Exception {
-        assertEquals("ONE", 
vs.findValue("@com.opensymphony.xwork2.ognl.MyNumbers@values()[0]", 
String.class));
-        assertEquals("TWO", 
vs.findValue("@com.opensymphony.xwork2.ognl.MyNumbers@values()[1]", 
String.class));
-        assertEquals("THREE", 
vs.findValue("@com.opensymphony.xwork2.ognl.MyNumbers@values()[2]", 
String.class));
+        assertNull("ONE", 
vs.findValue("@com.opensymphony.xwork2.ognl.MyNumbers@values()[0]", 
String.class));
+        assertNull("TWO", 
vs.findValue("@com.opensymphony.xwork2.ognl.MyNumbers@values()[1]", 
String.class));
+        assertNull("THREE", 
vs.findValue("@com.opensymphony.xwork2.ognl.MyNumbers@values()[2]", 
String.class));
     }
 
     public void testStaticMethodDisallow() {
diff --git 
a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java 
b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
index 03bad82e4..381b7d0ad 100644
--- 
a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
+++ 
b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
@@ -413,7 +413,7 @@ public class SecurityMemberAccessTest {
         boolean actual = sma.isAccessible(context, MyValues.class, values, 
null);
 
         // then
-        assertTrue("Access to enums is blocked!", actual);
+        assertFalse("Access to enums is allowed!", actual);
     }
 
     @Test