2015-09-14 21:53 GMT+02:00 Steven Willis :
> 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']"
[1]
http://struts.apache.org/docs/security.html#Security-Internalsecuritymechanism
Regards
--
Łukasz
+ 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
