Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Restrict accepted parameter name length
Thanks to Johno Crawford for the patch.

git-svn-id: https://svn.apache.org/repos/asf/struts/struts2/trunk@1368841 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
rgielen committed Aug 3, 2012
1 parent a1ca307 commit 80e0318
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->
Expand Down Expand Up @@ -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();
Expand All @@ -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>();
Expand All @@ -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--) {
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 80e0318

Please sign in to comment.