2015-09-14 21:53 GMT+02:00 Steven Willis <swil...@cargurus.com>:
> 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

There is no simple answer :) There was a lot of examples which were
using the 'new' keyword to create instances, ie. 'x=new
org.apache.struts2.ImportantObject(); #x.unsecureOperation()' and also
examples of accessing internal scopes, ie. '#' + 'application' and so
on. So we dropped support for spaces in param names 'just in case' :)

Also using a RegEx to filter them out it's just a temporary solution
and not the best one (I'm working on better solution to simple
distinguish origins of each parameter - external or internal).

And I think after we have introduced the Internal Security mechanism
[1] a lot of those RegExs are invalid now, but we afraid too much to
relax them :\

That being said, I think we should try change this pattern to allow
params like this "map['my key']"


+ 48 606 323 122 http://www.lenart.org.pl/

To unsubscribe, e-mail: user-unsubscr...@struts.apache.org
For additional commands, e-mail: user-h...@struts.apache.org

Reply via email to