CVE-2012-4387

@@ -95,9 +95,11 @@
  * <!-- START SNIPPET: parameters -->
  * <p/>
  * <ul>
- * <p/>
  * <li>ordered - set to true if you want the top-down property setter behaviour</li>
- * <p/>
+ * <li>acceptParamNames - a comma delimited list of regular expressions to describe a whitelist of accepted parameter names.
+ * Don't change the default unless you know what you are doing in terms of security implications</li>
+ * <li>excludeParams - a comma delimited list of regular expressions to describe a blacklist of not allowed parameter names</li>
+ * <li>paramNameMaxLength - the maximum length of parameter names; parameters with longer names will be ignored; the default is 100 characters</li>
  * </ul>
  * <p/>
  * <!-- END SNIPPET: parameters -->
@@ -129,6 +131,10 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
 
     private static final Logger LOG = LoggerFactory.getLogger(ParametersInterceptor.class);
 
+    protected static final int PARAM_NAME_MAX_LENGTH = 100;
+
+    private int paramNameMaxLength = PARAM_NAME_MAX_LENGTH;
+
     boolean ordered = false;
     Set<Pattern> excludeParams = Collections.emptySet();
     Set<Pattern> acceptParams = Collections.emptySet();
@@ -150,7 +156,16 @@ public static void setDevMode(String mode) {
         devMode = "true".equals(mode);
     }
 
-    public void setAcceptParamNames(String commaDelim) {
+	/**
+	 * Sets a comma-delimited list of regular expressions to match
+	 * parameters that are allowed in the parameter map (aka whitelist).
+	 * <p/>
+	 * Don't change the default unless you know what you are doing in terms
+	 * of security implications.
+	 *
+	 * @param commaDelim A comma-delimited list of regular expressions
+	 */
+	public void setAcceptParamNames(String commaDelim) {
         Collection<String> acceptPatterns = ArrayUtils.asCollection(commaDelim);
         if (acceptPatterns != null) {
             acceptParams = new HashSet<Pattern>();
@@ -160,6 +175,16 @@ public void setAcceptParamNames(String commaDelim) {
         }
     }
 
+    /**
+     * If the param name exceeds the configured maximum length it will not be
+     * accepted.
+     *
+     * @param paramNameMaxLength Maximum length of param names
+     */
+    public void setParamNameMaxLength(int paramNameMaxLength) {
+        this.paramNameMaxLength = paramNameMaxLength;
+    }
+
     static private int countOGNLCharacters(String s) {
         int count = 0;
         for (int i = s.length() - 1; i >= 0; i--) {
@@ -350,10 +375,15 @@ protected String getParameterLogMap(Map<String, Object> parameters) {
     }
 
     protected boolean acceptableName(String name) {
-        return isAccepted(name) && !isExcluded(name);
+        return isWithinLengthLimit(name) && isAccepted(name)
+                && !isExcluded(name);
     }
 
-    protected boolean isAccepted(String paramName) {
+	protected boolean isWithinLengthLimit( String name ) {
+		return name.length() <= paramNameMaxLength;
+	}
+
+	protected boolean isAccepted(String paramName) {
         if (!this.acceptParams.isEmpty()) {
             for (Pattern pattern : acceptParams) {
                 Matcher matcher = pattern.matcher(paramName);

@@ -201,7 +201,39 @@ public void testParametersWithSpacesInTheName() throws Exception {
         assertEquals(0, existingMap.size());
     }
 
-    public void testExcludedTrickyParameters() throws Exception {
+    public void testLargeParameterNameWithDefaultLimit() throws Exception {
+		ParametersInterceptor parametersInterceptor = createParametersInterceptor();
+		doTestParameterNameLengthRestriction(parametersInterceptor, ParametersInterceptor.PARAM_NAME_MAX_LENGTH);
+    }
+
+	public void testLargeParameterNameWithCustomLimit() throws Exception {
+		ParametersInterceptor parametersInterceptor = createParametersInterceptor();
+		int limit = 20;
+		parametersInterceptor.setParamNameMaxLength(limit);
+		doTestParameterNameLengthRestriction(parametersInterceptor, limit);
+	}
+
+	private void doTestParameterNameLengthRestriction( ParametersInterceptor parametersInterceptor,
+													   int paramNameMaxLength ) {
+		StringBuilder sb = new StringBuilder();
+		for (int i = 0; i < paramNameMaxLength + 1; i++) {
+            sb.append("x");
+        }
+
+		Map<String, Object> actual = new LinkedHashMap<String, Object>();
+		parametersInterceptor.setValueStackFactory(createValueStackFactory(actual));
+		ValueStack stack = createStubValueStack(actual);
+
+		Map<String, Object> parameters = new HashMap<String, Object>();
+		parameters.put(sb.toString(), "");
+		parameters.put("huuhaa", "");
+
+		Action action = new SimpleAction();
+		parametersInterceptor.setParameters(action, stack, parameters);
+		assertEquals(1, actual.size());
+	}
+
+	public void testExcludedTrickyParameters() throws Exception {
         Map<String, Object> params = new HashMap<String, Object>() {
             {
                 put("blah", "This is blah");

@@ -96,9 +96,11 @@
  * <!-- START SNIPPET: parameters -->
  * <p/>
  * <ul>
- * <p/>
  * <li>ordered - set to true if you want the top-down property setter behaviour</li>
- * <p/>
+ * <li>acceptParamNames - a comma delimited list of regular expressions to describe a whitelist of accepted parameter names.
+ * Don't change the default unless you know what you are doing in terms of security implications</li>
+ * <li>excludeParams - a comma delimited list of regular expressions to describe a blacklist of not allowed parameter names</li>
+ * <li>paramNameMaxLength - the maximum length of parameter names; parameters with longer names will be ignored; the default is 100 characters</li>
  * </ul>
  * <p/>
  * <!-- END SNIPPET: parameters -->
@@ -130,6 +132,10 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
 
     private static final Logger LOG = LoggerFactory.getLogger(ParametersInterceptor.class);
 
+    protected static final int PARAM_NAME_MAX_LENGTH = 100;
+
+    private int paramNameMaxLength = PARAM_NAME_MAX_LENGTH;
+
     boolean ordered = false;
     Set<Pattern> excludeParams = Collections.emptySet();
     Set<Pattern> acceptParams = Collections.emptySet();
@@ -151,7 +157,16 @@ public static void setDevMode(String mode) {
         devMode = "true".equals(mode);
     }
 
-    public void setAcceptParamNames(String commaDelim) {
+	/**
+	 * Sets a comma-delimited list of regular expressions to match
+	 * parameters that are allowed in the parameter map (aka whitelist).
+	 * <p/>
+	 * Don't change the default unless you know what you are doing in terms
+	 * of security implications.
+	 *
+	 * @param commaDelim A comma-delimited list of regular expressions
+	 */
+	public void setAcceptParamNames(String commaDelim) {
         Collection<String> acceptPatterns = ArrayUtils.asCollection(commaDelim);
         if (acceptPatterns != null) {
             acceptParams = new HashSet<Pattern>();
@@ -161,6 +176,16 @@ public void setAcceptParamNames(String commaDelim) {
         }
     }
 
+    /**
+     * If the param name exceeds the configured maximum length it will not be
+     * accepted.
+     *
+     * @param paramNameMaxLength Maximum length of param names
+     */
+    public void setParamNameMaxLength(int paramNameMaxLength) {
+        this.paramNameMaxLength = paramNameMaxLength;
+    }
+
     static private int countOGNLCharacters(String s) {
         int count = 0;
         for (int i = s.length() - 1; i >= 0; i--) {
@@ -351,10 +376,15 @@ protected String getParameterLogMap(Map<String, Object> parameters) {
     }
 
     protected boolean acceptableName(String name) {
-        return isAccepted(name) && !isExcluded(name);
+        return isWithinLengthLimit(name) && isAccepted(name)
+                && !isExcluded(name);
     }
 
-    protected boolean isAccepted(String paramName) {
+	protected boolean isWithinLengthLimit( String name ) {
+		return name.length() <= paramNameMaxLength;
+	}
+
+	protected boolean isAccepted(String paramName) {
         if (!this.acceptParams.isEmpty()) {
             for (Pattern pattern : acceptParams) {
                 Matcher matcher = pattern.matcher(paramName);

@@ -201,7 +201,39 @@ public void testParametersWithSpacesInTheName() throws Exception {
         assertEquals(0, existingMap.size());
     }
 
-    public void testExcludedTrickyParameters() throws Exception {
+    public void testLargeParameterNameWithDefaultLimit() throws Exception {
+		ParametersInterceptor parametersInterceptor = createParametersInterceptor();
+		doTestParameterNameLengthRestriction(parametersInterceptor, ParametersInterceptor.PARAM_NAME_MAX_LENGTH);
+    }
+
+	public void testLargeParameterNameWithCustomLimit() throws Exception {
+		ParametersInterceptor parametersInterceptor = createParametersInterceptor();
+		int limit = 20;
+		parametersInterceptor.setParamNameMaxLength(limit);
+		doTestParameterNameLengthRestriction(parametersInterceptor, limit);
+	}
+
+	private void doTestParameterNameLengthRestriction( ParametersInterceptor parametersInterceptor,
+													   int paramNameMaxLength ) {
+		StringBuilder sb = new StringBuilder();
+		for (int i = 0; i < paramNameMaxLength + 1; i++) {
+            sb.append("x");
+        }
+
+		Map<String, Object> actual = new LinkedHashMap<String, Object>();
+		parametersInterceptor.setValueStackFactory(createValueStackFactory(actual));
+		ValueStack stack = createStubValueStack(actual);
+
+		Map<String, Object> parameters = new HashMap<String, Object>();
+		parameters.put(sb.toString(), "");
+		parameters.put("huuhaa", "");
+
+		Action action = new SimpleAction();
+		parametersInterceptor.setParameters(action, stack, parameters);
+		assertEquals(1, actual.size());
+	}
+
+	public void testExcludedTrickyParameters() throws Exception {
         Map<String, Object> params = new HashMap<String, Object>() {
             {
                 put("blah", "This is blah");