struts-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "bruce liu (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (WW-4146) cache attack at OgnlUtil.expressions
Date Thu, 25 Jul 2013 05:09:48 GMT

    [ https://issues.apache.org/jira/browse/WW-4146?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13719235#comment-13719235
] 

bruce liu edited comment on WW-4146 at 7/25/13 5:09 AM:
--------------------------------------------------------

to [Maurizio Cucchiara], i said "once i used LRUMap in an CTI system to cache telnumber and
customer information" . i won't use LRUMap for this issue, no matter Guava or Commons implementation,
it is no sense if Struts don't filter invalid parameteres .

in class OgnlUtil:

protected void setValue(String name, Map<String, Object> context, Object root, Object
value, boolean evalName) throws OgnlException {
	Object tree = compile(name, context);
	if (!evalName && isEvalExpression(tree, context)) {
		throw new OgnlException("Eval expression cannot be used as parameter name");
	}
	Ognl.setValue(tree, context, root, value);
}

if name is invalid, method call "Ognl.setValue(tree, context, root, value)"  will throw an
exception, we can move "expressions.putIfAbsent(expression, tree);" from compile method to
line after Ognl.setValue.

so that, invalid parameter won't be cached, the side effect is we has to parse every invalid
parameter every time. valid parameter will still parse only one time.

                
      was (Author: coderbee):
    to [Maurizio Cucchiara], i said "once i used LRUMap in an CTI system to cache telnumber
and customer information" . i won't use LRUMap for this issue, no matter Guava or Commons
implementation, it is no sense if Struts don't filter invalid parameteres .

in class OgnlUtil:

protected void setValue(String name, Map<String, Object> context, Object root, Object
value, boolean evalName) throws OgnlException {
	Object tree = compile(name, context);
	if (!evalName && isEvalExpression(tree, context)) {
		throw new OgnlException("Eval expression cannot be used as parameter name");
	}
	Ognl.setValue(tree, context, root, value);
}

if name is invalid, method call "Ognl.setValue(tree, context, root, value)"  will throw an
exception, we can move "expressions.putIfAbsent(expression, tree);" from compile method to
line after Ognl.setValue.

so that, invalid parameter won't be cached, the side effect is we will every invalid parameter
every time. valid parameter will still parse only one time.

                  
> cache attack at  OgnlUtil.expressions
> -------------------------------------
>
>                 Key: WW-4146
>                 URL: https://issues.apache.org/jira/browse/WW-4146
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Expression Language
>    Affects Versions: 2.3.15.1
>            Reporter: bruce liu
>             Fix For: 2.3.17
>
>         Attachments: WW-4146.patch
>
>
> in class com.opensymphony.xwork2.ognl.OgnlUtil, code :
> {code:java}
> tree = expressions.get(expression);
> if (tree == null) {
> 	tree = Ognl.parseExpression(expression);
> 	expressions.putIfAbsent(expression, tree);
> }
> {code}
> every parameter in the request cached in  field expressions  which is an instances of
ConcurrentMap<String, Object>, use parameterName as key. so i construct huge different
parameters that has different name (like  "abc[123],  abc[124]" ), they all cached in  expressions,
this cause outofmemory error, and let map acted like a list .

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message