struts-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Steven Willis <swil...@cargurus.com>
Subject Characters allowed in map keys in parameters
Date Mon, 14 Sep 2015 19:53:22 GMT
I've been having issues with map keys in struts and I finally tracked it down to the pattern
here:

    https://github.com/apache/struts/blob/master/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java#L19


Which is:

    "\\w+((\\.\\w+)|(\\[\\d+\\])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'\\])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*"

This apparently restricts map keys to strings of word characters [a-zA-Z0-9_] or virtually
all of the 20,950 characters in the CJK Unified Ideographs (why does it stop at 9FA5 and not
9FFF or 9FD5?). Is there some justification for which characters were allowed here, and why
things like spaces and slashes are excluded? It seems like you could allow anything except
for single quotes and you'd be safe, right?

I've read through all of the following which seemed to be either directly or indirectly related
to the issue:

    https://issues.apache.org/jira/browse/WW-4250

    http://markmail.org/message/y7d6hgftyf2jauz5

    https://cwiki.apache.org/confluence/display/WW/S2-003
    https://cwiki.apache.org/confluence/display/WW/S2-005

    https://cwiki.apache.org/confluence/display/WW/S2-008
    https://cwiki.apache.org/confluence/display/WW/S2-009
    https://issues.apache.org/jira/browse/WW-3729
    https://issues.apache.org/jira/browse/WW-3668
    https://issues.apache.org/jira/browse/WW-4257

Some of the messages say that allowing spaces is a security vulnerability, but how could that
be? Isn't foo['hello world'] and foo['hello/world'] just as safe as foo['hello_world'] ? The
actual security vulnerabilities seem related to other forms parameter values (using #, or
forms that aren't inside single quoted strings).


This commit:

    https://github.com/apache/struts/commit/8a93df10c4f5f3f22f1837c47b4ca9b4facc4f94#diff-d6b23e0dce6eef0d9662cbfacbc8c916L376

Changed the testParametersWithSpacesInTheName test to expect spaces not to work rather than
to work. But the commit message doesn't explain why.

Actually looks like the test has flipped back and forth between expecting spaces to work and
not work a few times. I just found what I believe is the earliest commit that has a reference
to not accepting spaces (only accessible via tags that are not ancestors of master):

    https://github.com/apache/struts/commit/41f90ae39d0783f64641726e7e6b4741663c04bd#diff-d6b23e0dce6eef0d9662cbfacbc8c916

That commit also doesn't explain why spaces shouldn't be allowed.

-Steven Willis
Mime
View raw message