[struts] 01/01: WW-5354 Ensure ActionSupport fields are not parameter injectable

[kusal]

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

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

commit 0432205a684cd6b31c790f309fc908192c0f73d7
Author: Kusal Kithul-Godage <g...@kusal.io>
AuthorDate: Thu Oct 12 21:31:16 2023 +1100

    WW-5354 Ensure ActionSupport fields are not parameter injectable
---
 .../security/DefaultExcludedPatternsChecker.java   | 23 +++++-----------------
 .../interceptor/ParametersInterceptorTest.java     | 16 ++++++++++++++-
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git 
a/core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java
 
b/core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java
index 276ab92a5..cf425d67c 100644
--- 
a/core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java
+++ 
b/core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java
@@ -36,8 +36,9 @@ public class DefaultExcludedPatternsChecker implements 
ExcludedPatternsChecker {
     private static final Logger LOG = 
LogManager.getLogger(DefaultExcludedPatternsChecker.class);
 
     public static final String[] EXCLUDED_PATTERNS = {
-        
"(^|\\%\\{)((#?)(top(\\.|\\['|\\[\")|\\[\\d\\]\\.)?)(dojo|struts|session|request|response|application|servlet(Request|Response|Context)|parameters|context|_memberAccess)(\\.|\\[).*",
-        ".*(^|\\.|\\[|\\'|\"|get)class(\\(\\.|\\[|\\'|\").*"
+            
"(^|\\%\\{)((#?)(top(\\.|\\['|\\[\")|\\[\\d\\]\\.)?)(dojo|struts|session|request|response|application|servlet(Request|Response|Context)|parameters|context|_memberAccess)(\\.|\\[).*",
+            ".*(^|\\.|\\[|\\'|\"|get)class(\\(\\.|\\[|\\'|\").*",
+            "actionErrors|actionMessages|fieldErrors"
     };
 
     private Set<Pattern> excludedPatterns;
@@ -48,21 +49,7 @@ public class DefaultExcludedPatternsChecker implements 
ExcludedPatternsChecker {
 
     @Inject(value = StrutsConstants.STRUTS_OVERRIDE_EXCLUDED_PATTERNS, 
required = false)
     protected void setOverrideExcludePatterns(String excludePatterns) {
-        if (excludedPatterns != null && excludedPatterns.size() > 0) {
-            LOG.warn("Overriding excluded patterns [{}] with [{}], be aware 
that this affects all instances and safety of your application!",
-                        excludedPatterns, excludePatterns);
-        } else {
-            // Limit unwanted log entries (when excludedPatterns null/empty - 
usually 1st call)
-            LOG.debug("Overriding excluded patterns with [{}]", 
excludePatterns);
-        }
-        excludedPatterns = new HashSet<>();
-        try {
-            for (String pattern : 
TextParseUtil.commaDelimitedStringToSet(excludePatterns)) {
-                excludedPatterns.add(Pattern.compile(pattern, 
Pattern.CASE_INSENSITIVE));
-            }
-        } finally {
-            excludedPatterns = Collections.unmodifiableSet(excludedPatterns);
-        }
+        setExcludedPatterns(excludePatterns);
     }
 
     @Inject(value = StrutsConstants.STRUTS_ADDITIONAL_EXCLUDED_PATTERNS, 
required = false)
@@ -98,7 +85,7 @@ public class DefaultExcludedPatternsChecker implements 
ExcludedPatternsChecker {
 
     @Override
     public void setExcludedPatterns(Set<String> patterns) {
-        if (excludedPatterns != null && excludedPatterns.size() > 0) {
+        if (excludedPatterns != null && !excludedPatterns.isEmpty()) {
             LOG.warn("Replacing excluded patterns [{}] with [{}], be aware 
that this affects all instances and safety of your application!",
                         excludedPatterns, patterns);
         } else {
diff --git 
a/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
 
b/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
index 6f28dae07..2e7a0be16 100644
--- 
a/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
+++ 
b/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
@@ -21,6 +21,7 @@ package com.opensymphony.xwork2.interceptor;
 import com.opensymphony.xwork2.Action;
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.ActionProxy;
+import com.opensymphony.xwork2.ActionSupport;
 import com.opensymphony.xwork2.ModelDrivenAction;
 import com.opensymphony.xwork2.SimpleAction;
 import com.opensymphony.xwork2.TestBean;
@@ -300,7 +301,7 @@ public class ParametersInterceptorTest extends 
XWorkTestCase {
             }
         };
 
-        final Map<String, Boolean> excluded = new HashMap<String, Boolean>();
+        final Map<String, Boolean> excluded = new HashMap<>();
         ParametersInterceptor pi = new ParametersInterceptor() {
 
             @Override
@@ -364,6 +365,19 @@ public class ParametersInterceptorTest extends 
XWorkTestCase {
         assertEquals("This is blah", ((SimpleAction) 
proxy.getAction()).getBlah());
     }
 
+    public void testActionSupportParametersBlocked() throws Exception {
+        Map<String, Object> params = new HashMap<>();
+        params.put("actionErrors", "fakeError");
+        params.put("actionMessages", "fakeMessage");
+
+        ActionContext extraContext = 
ActionContext.of().withParameters(HttpParameters.create(params).build());
+
+        ActionProxy proxy = actionProxyFactory.createActionProxy("", 
MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME, null, 
extraContext.getContextMap());
+        proxy.execute();
+        assertEquals(0, ((ActionSupport) 
proxy.getAction()).getActionMessages().size());
+        assertEquals(0, ((ActionSupport) 
proxy.getAction()).getActionErrors().size());
+    }
+
     public void testParametersWithSpacesInTheName() throws Exception {
         Map<String, Object> params = new HashMap<>();
         params.put("theProtectedMap['p0 p1']", "test1");