WW-5354 Ensure ActionSupport fields are not parameter injectable

SimaSaasDemo · Oct 12, 2023 · 0432205 · 0432205
1 parent 23feab6
commit 0432205
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 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
@@ -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 DefaultExcludedPatternsChecker() {
 
     @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 void setExcludedPatterns(String[] patterns) {
 
     @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
@@ -21,6 +21,7 @@
 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 void testArrayClassPollutionBlockedByPattern() throws Exception {
             }
         };
 
-        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 void testParameters() throws Exception {
         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");