Author: lukaszlenart Date: Tue Dec 27 22:34:37 2011 New Revision: 1225038 URL: http://svn.apache.org/viewvc?rev=1225038&view=rev Log: Merges changes from 2.3.x branch
Modified: struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/util/ArrayUtils.java struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java Modified: struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java?rev=1225038&r1=1225037&r2=1225038&view=diff ============================================================================== --- struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java (original) +++ struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java Tue Dec 27 22:34:37 2011 @@ -21,13 +21,6 @@ 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; @@ -35,6 +28,14 @@ import com.opensymphony.xwork2.util.Text 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 @@ import com.opensymphony.xwork2.util.logg * 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 A 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 class CookieInterceptor extends A 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 class CookieInterceptor extends A 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 @@ public class CookieInterceptor extends A 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); } Modified: struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java?rev=1225038&r1=1225037&r2=1225038&view=diff ============================================================================== --- struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java (original) +++ struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java Tue Dec 27 22:34:37 2011 @@ -21,10 +21,10 @@ import com.opensymphony.xwork2.Validatio 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 exten 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 class ParametersInterceptor exten } 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 @@ public class ParametersInterceptor exten * @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 class ParametersInterceptor exten } } - /** - * 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); - } - } Modified: struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/util/ArrayUtils.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/util/ArrayUtils.java?rev=1225038&r1=1225037&r2=1225038&view=diff ============================================================================== --- struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/util/ArrayUtils.java (original) +++ struts/struts2/trunk/xwork-core/src/main/java/com/opensymphony/xwork2/util/ArrayUtils.java Tue Dec 27 22:34:37 2011 @@ -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 class ArrayUtils { 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; + } } Modified: struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java URL: http://svn.apache.org/viewvc/struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java?rev=1225038&r1=1225037&r2=1225038&view=diff ============================================================================== --- struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java (original) +++ struts/struts2/trunk/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java Tue Dec 27 22:34:37 2011 @@ -196,11 +196,7 @@ public class ParametersInterceptorTest e 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 {