WW-5839 Reject unconfined wildcard substitution into class= attributes

core/src/main/java/org/apache/struts2/config/impl/AbstractMatcher.java

@@ -19,6 +19,7 @@
 package org.apache.struts2.config.impl;
 
 import org.apache.struts2.util.PatternMatcher;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.math.NumberUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -46,6 +47,13 @@ public abstract class AbstractMatcher<E> implements Serializable {
     private static final Logger LOG = LogManager.getLogger(AbstractMatcher.class);
     private static final Pattern WILDCARD_PATTERN = Pattern.compile("\\{(.)}");
 
+    /**
+     * Characters a wildcard-matched segment may contain when it is substituted into a class name:
+     * Unicode letters and digits, '_' and '$'. Excludes '.', '/' and everything else that could move
+     * the resulting class name out of the package the mapping's author wrote.
+     */
+    private static final Pattern SAFE_CLASS_NAME_SEGMENT = Pattern.compile("[\\p{L}\\p{Nd}_$]*");
+
     /**
      * <p> Handles all wildcard pattern matching. </p>
      */
@@ -207,13 +215,40 @@ protected String convertParam(String val, Map<String, String> vars) {
 
         StringBuilder result = new StringBuilder();
         while (wildcardMatcher.find()) {
-            wildcardMatcher.appendReplacement(result, vars.getOrDefault(wildcardMatcher.group(1), ""));
+            wildcardMatcher.appendReplacement(result, Matcher.quoteReplacement(vars.getOrDefault(wildcardMatcher.group(1), "")));
         }
         wildcardMatcher.appendTail(result);
 
         return result.toString();
     }
 
+    /**
+     * <p> Checks that every wildcard-matched value that {@link #convertParam(String, Map)} would
+     * substitute into {@code val} is safe to use as part of a class name. Callers reject the
+     * whole match when this returns {@code false}, as they do for a disallowed method name. </p>
+     *
+     * @param val  The class name template, may be {@code null}
+     * @param vars A Map of wildcard-matched strings
+     * @return {@code false} if a substituted value contains a character other than a letter,
+     * digit, '_' or '$'
+     */
+    protected boolean isSafeForClassName(String val, Map<String, String> vars) {
+        if (val == null) {
+            return true;
+        }
+
+        Matcher wildcardMatcher = WILDCARD_PATTERN.matcher(val);
+        while (wildcardMatcher.find()) {
+            String value = vars.getOrDefault(wildcardMatcher.group(1), "");
+            if (!SAFE_CLASS_NAME_SEGMENT.matcher(value).matches()) {
+                LOG.warn("Wildcard-matched value [{}] is not allowed in class name [{}]; match rejected",
+                        StringUtils.normalizeSpace(value), val);
+                return false;
+            }
+        }
+        return true;
+    }
+
     /**
      * <p> Stores a compiled wildcard pattern and the object it came
      * from. </p>

core/src/main/java/org/apache/struts2/config/impl/ActionConfigMatcher.java

@@ -114,6 +114,10 @@ public ActionConfig convert(String path, ActionConfig orig,
             return null;
         }
 
+        if (!hasSafeClassNames(orig, vars)) {
+            return null;
+        }
+
         String className = convertParam(orig.getClassName(), vars);
         String pkgName = convertParam(orig.getPackageName(), vars);
 
@@ -152,4 +156,21 @@ public ActionConfig convert(String path, ActionConfig orig,
                 .location(orig.getLocation())
                 .build();
     }
+
+    private boolean hasSafeClassNames(ActionConfig orig, Map<String, String> vars) {
+        if (!isSafeForClassName(orig.getClassName(), vars)) {
+            return false;
+        }
+        for (ResultConfig result : orig.getResults().values()) {
+            if (!isSafeForClassName(result.getClassName(), vars)) {
+                return false;
+            }
+        }
+        for (ExceptionMappingConfig ex : orig.getExceptionMappings()) {
+            if (!isSafeForClassName(ex.getExceptionClassName(), vars)) {
+                return false;
+            }
+        }
+        return true;
+    }
 }

core/src/test/java/org/apache/struts2/config/impl/WildcardClassNameHardeningTest.java

@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.config.impl;
+
+import org.apache.struts2.XWorkTestCase;
+import org.apache.struts2.config.entities.ActionConfig;
+import org.apache.struts2.config.entities.ExceptionMappingConfig;
+import org.apache.struts2.config.entities.ResultConfig;
+import org.apache.struts2.util.WildcardHelper;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+public class WildcardClassNameHardeningTest extends XWorkTestCase {
+
+    private ActionConfigMatcher matcherFor(ActionConfig config) {
+        Map<String, ActionConfig> map = new HashMap<>();
+        map.put(config.getName(), config);
+        return new ActionConfigMatcher(new WildcardHelper(), map, false);
+    }
+
+    public void testActionClassNameRejectsSegmentWithDot() {
+        ActionConfigMatcher matcher = matcherFor(new ActionConfig.Builder("package", "*", "actions.{1}Action")
+                .methodName("execute")
+                .build());
+
+        assertNull(matcher.match("util.Harmless"));
+        assertEquals("actions.UserAction", matcher.match("User").getClassName());
+    }
+
+    public void testResultClassNameRejectsSegmentWithDot() {
+        ActionConfigMatcher matcher = matcherFor(new ActionConfig.Builder("package", "*", "actions.FixedAction")
+                .addResultConfig(new ResultConfig.Builder("success", "results.{1}Result").build())
+                .build());
+
+        assertNull(matcher.match("util.Harmless"));
+        assertEquals("results.UserResult", matcher.match("User").getResults().get("success").getClassName());
+    }
+
+    public void testExceptionMappingClassNameRejectsSegmentWithDot() {
+        ActionConfigMatcher matcher = matcherFor(new ActionConfig.Builder("package", "*", "actions.FixedAction")
+                .addExceptionMapping(new ExceptionMappingConfig.Builder("ex", "errors.{1}Exception", "error").build())
+                .build());
+
+        assertNull(matcher.match("util.Harmless"));
+        assertEquals("errors.UserException", matcher.match("User").getExceptionMappings().get(0).getExceptionClassName());
+    }
+
+    public void testClassNameAcceptsNonAsciiIdentifierAndDollar() {
+        ActionConfigMatcher matcher = matcherFor(new ActionConfig.Builder("package", "*", "actions.{1}Action")
+                .build());
+
+        assertEquals("actions.ÜserAction", matcher.match("Üser").getClassName());
+        assertEquals("actions.Inner$1Action", matcher.match("Inner$1").getClassName());
+    }
+
+    public void testDollarAndBackslashAreSubstitutedLiterally() {
+        ActionConfigMatcher matcher = matcherFor(new ActionConfig.Builder("package", "*", "actions.FixedAction")
+                .addParam("value", "{1}")
+                .build());
+
+        assertEquals("a$1b", matcher.match("a$1b").getParams().get("value"));
+        assertEquals("a\\b", matcher.match("a\\b").getParams().get("value"));
+    }
+
+    public void testMethodNameWildcardIsGated() {
+        ActionConfigMatcher matcher = matcherFor(new ActionConfig.Builder("package", "run/*", "actions.RunAction")
+                .methodName("{1}")
+                .addAllowedMethod(Set.of("execute"))
+                .setStrictMethodInvocation(true)
+                .build());
+
+        assertNull(matcher.match("run/deleteAll"));
+        assertEquals("execute", matcher.match("run/execute").getMethodName());
+    }
+}