CVE-2012-0394

@@ -21,20 +21,21 @@
 
 package org.apache.struts2.interceptor;
 
-import java.util.*;
-
-import javax.servlet.http.Cookie;
-import javax.servlet.http.HttpServletRequest;
-
-import org.apache.struts2.ServletActionContext;
-
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.ActionInvocation;
 import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
 import com.opensymphony.xwork2.util.TextParseUtil;
 import com.opensymphony.xwork2.util.ValueStack;
 import com.opensymphony.xwork2.util.logging.Logger;
 import com.opensymphony.xwork2.util.logging.LoggerFactory;
+import org.apache.struts2.ServletActionContext;
+
+import javax.servlet.http.Cookie;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Pattern;
 
 /**
  * <!-- START SNIPPET: description -->
@@ -75,7 +76,8 @@
  *                                                         action. If more than one cookie name is desired it could be
  *                                                         comma-separated. If left empty, it will assume any value would
  *                                                         be ok. If more than one value is specified (comma-separated)
- *                                                         it will assume a match if either value is matched.
+ *                                                         it will assume a match if either value is matched.</li>
+ *     <li>acceptCookieNames (optional) - Pattern used to check if name of cookie matches the provided patter, to </li>
  * </ul>
  *
  * <!-- END SNIPPET: parameters -->
@@ -161,9 +163,14 @@ public class CookieInterceptor extends AbstractInterceptor {
 
     private static final Logger LOG = LoggerFactory.getLogger(CookieInterceptor.class);
 
+    private static final String ACCEPTED_PATTERN = "[a-zA-Z0-9\\.\\]\\[_'\\s]+";
+
     private Set<String> cookiesNameSet = Collections.emptySet();
     private Set<String> cookiesValueSet = Collections.emptySet();
 
+    // Allowed names of cookies
+    private Pattern acceptedPattern = Pattern.compile(ACCEPTED_PATTERN);
+
     /**
      * Set the <code>cookiesName</code> which if matched will allow the cookie
      * to be injected into action, could be comma-separated string.
@@ -187,11 +194,20 @@ public void setCookiesValue(String cookiesValue) {
             this.cookiesValueSet = TextParseUtil.commaDelimitedStringToSet(cookiesValue);
     }
 
+    /**
+     * Set the <code>acceptCookieNames</code> pattern of allowed names of cookies to protect against remote command execution vulnerability
+     *
+     * @param pattern used to check cookie name against
+     */
+    public void setAcceptCookieNames(String pattern) {
+        acceptedPattern = Pattern.compile(pattern);
+    }
+
     public String intercept(ActionInvocation invocation) throws Exception {
         if (LOG.isDebugEnabled()) {
             LOG.debug("start interception");
         }
-        
+
         // contains selected cookies
         final Map<String, String> cookiesMap = new LinkedHashMap<String, String>();
 
@@ -203,13 +219,17 @@ public String intercept(ActionInvocation invocation) throws Exception {
                 String name = cookie.getName();
                 String value = cookie.getValue();
 
-                if (cookiesNameSet.contains("*")) {
-                    if (LOG.isDebugEnabled()) {
-                        LOG.debug("contains cookie name [*] in configured cookies name set, cookie with name [" + name + "] with value [" + value + "] will be injected");
+                if (acceptedPattern.matcher(name).matches()) {
+                    if (cookiesNameSet.contains("*")) {
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("contains cookie name [*] in configured cookies name set, cookie with name [" + name + "] with value [" + value + "] will be injected");
+                        }
+                        populateCookieValueIntoStack(name, value, cookiesMap, stack);
+                    } else if (cookiesNameSet.contains(cookie.getName())) {
+                        populateCookieValueIntoStack(name, value, cookiesMap, stack);
                     }
-                    populateCookieValueIntoStack(name, value, cookiesMap, stack);
-                } else if (cookiesNameSet.contains(cookie.getName())) {
-                    populateCookieValueIntoStack(name, value, cookiesMap, stack);
+                } else {
+                    LOG.warn("Cookie name [" + name + "] does not match accepted cookie names pattern [" + acceptedPattern + "]");
                 }
             }
         }
@@ -251,7 +271,7 @@ else if (cookiesValueSet.contains("*"))
                 if (LOG.isDebugEnabled()) {
                     LOG.debug("both configured cookie name and value matched, cookie ["+cookieName+"] with value ["+cookieValue+"] will be injected");
                 }
-                
+
                 cookiesMap.put(cookieName, cookieValue);
                 stack.setValue(cookieName, cookieValue);
             }

@@ -21,10 +21,10 @@
 import com.opensymphony.xwork2.conversion.impl.InstantiatingNullHandler;
 import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
 import com.opensymphony.xwork2.inject.Inject;
+import com.opensymphony.xwork2.util.ArrayUtils;
 import com.opensymphony.xwork2.util.ClearableValueStack;
 import com.opensymphony.xwork2.util.LocalizedTextUtil;
 import com.opensymphony.xwork2.util.MemberAccessValueStack;
-import com.opensymphony.xwork2.util.TextParseUtil;
 import com.opensymphony.xwork2.util.ValueStack;
 import com.opensymphony.xwork2.util.ValueStackFactory;
 import com.opensymphony.xwork2.util.logging.Logger;
@@ -135,7 +135,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
     static boolean devMode = false;
 
     // Allowed names of parameters
-    private String acceptedParamNames = "[a-zA-Z0-9\\.\\]\\[\\(\\)_'\\s]+";
+    private String acceptedParamNames = "[a-zA-Z0-9\\.\\]\\[\\(\\)_']+";
     private Pattern acceptedPattern = Pattern.compile(acceptedParamNames);
 
     private ValueStackFactory valueStackFactory;
@@ -151,7 +151,7 @@ public static void setDevMode(String mode) {
     }
 
     public void setAcceptParamNames(String commaDelim) {
-        Collection<String> acceptPatterns = asCollection(commaDelim);
+        Collection<String> acceptPatterns = ArrayUtils.asCollection(commaDelim);
         if (acceptPatterns != null) {
             acceptParams = new HashSet<Pattern>();
             for (String pattern : acceptPatterns) {
@@ -413,7 +413,7 @@ protected Set getExcludeParamsSet() {
      * @param commaDelim A comma-delimited list of regular expressions
      */
     public void setExcludeParams(String commaDelim) {
-        Collection<String> excludePatterns = asCollection(commaDelim);
+        Collection<String> excludePatterns = ArrayUtils.asCollection(commaDelim);
         if (excludePatterns != null) {
             excludeParams = new HashSet<Pattern>();
             for (String pattern : excludePatterns) {
@@ -422,17 +422,4 @@ public void setExcludeParams(String commaDelim) {
         }
     }
 
-    /**
-     * Return a collection from the comma delimited String.
-     *
-     * @param commaDelim the comma delimited String.
-     * @return A collection from the comma delimited String. Returns <tt>null</tt> if the string is empty.
-     */
-    private Collection<String> asCollection(String commaDelim) {
-        if (commaDelim == null || commaDelim.trim().length() == 0) {
-            return null;
-        }
-        return TextParseUtil.commaDelimitedStringToSet(commaDelim);
-    }
-
 }

@@ -15,6 +15,11 @@
  */
 package com.opensymphony.xwork2.util;
 
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
 /**
  * @author Dan Oxlade, dan d0t oxlade at gmail d0t c0m
  */
@@ -28,4 +33,22 @@ public static boolean isNotEmpty(Object[] array) {
         return !isEmpty(array);
     }
 
+    /**
+     * Return a collection from the comma delimited String.
+     *
+     * @param commaDelim the comma delimited String.
+     * @return A collection from the comma delimited String. Returns <tt>null</tt> if the string is empty.
+     */
+    public static Collection<String> asCollection(String commaDelim) {
+        if (commaDelim == null || commaDelim.trim().length() == 0) {
+            return null;
+        }
+        return TextParseUtil.commaDelimitedStringToSet(commaDelim);
+    }
+
+    public static <T> Set<T> asSet(T... element) {
+        HashSet<T> elements = new HashSet<T>(element.length);
+        Collections.addAll(elements, element);
+        return elements;
+    }
 }

@@ -196,11 +196,7 @@ public void testParametersWithSpacesInTheName() throws Exception {
         ActionProxy proxy = actionProxyFactory.createActionProxy("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME, extraContext);
         proxy.execute();
         Map<String, String> existingMap =  ((SimpleAction) proxy.getAction()).getTheProtectedMap();
-        assertEquals(4, existingMap.size());
-        assertEquals("test1", existingMap.get("p0 p1"));
-        assertEquals("test2", existingMap.get("p0p1 "));
-        assertEquals("test3", existingMap.get(" p0p1 "));
-        assertEquals("test4", existingMap.get(" p0 p1 "));
+        assertEquals(0, existingMap.size());
     }
 
     public void testExcludedTrickyParameters() throws Exception {

@@ -17,11 +17,17 @@
 
 import com.opensymphony.xwork2.util.location.Located;
 import com.opensymphony.xwork2.util.location.Location;
+import org.apache.commons.lang.StringUtils;
 
 import java.io.Serializable;
-import java.util.*;
-
-import org.apache.commons.lang.StringUtils;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
 
 /**
@@ -218,6 +224,7 @@ public static class Builder implements InterceptorListHolder{
 
         public Builder(ActionConfig toClone) {
             target = new ActionConfig(toClone);
+            addAllowedMethod(toClone.getAllowedMethods());
         }
 
         public Builder(String packageName, String name, String className) {