struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ing. Andrea Vettori" <>
Subject Re: Preventing OGNL evaluations of user input (was Re: Struts 2 performance)
Date Mon, 16 Jul 2007 14:40:29 GMT
The "Musachy" patch that prevents the parameters to have the pattern % 
{*} works great as a workaround (and i'm using that in my e-commerce  
site where I found this problem).

I think that the final patch should address the unlikely but possible  
case when the user has to enter (or we need to pass to an action)  
something like %{*} as a parameter. As discussed in the jira ticket I  
think that the patch should (a) prevent infinite recursion (as  
circular reference as you said) and (b) prevent the user input to be  
evaluated as expressions. If loopCount is the recursion counter, (a)  
is solved for any limited value but it limits the complexity of the  
expressions. I suggested the value can be parametrized so if one  
known he use complex expression can use a higher value. (b) is solved  
using loopCount=1 by default when dealing with user input.

Il giorno 16/lug/07, alle ore 16:31, Antonio Petrelli ha scritto:

> Probably there was a misunderstanding Andrea.
> First of all, are we talking about:
> 1. the "preliminar" patch, that at least prevents remote exploit by
> disallowing malicious code,
> or
> 2. the final patch? In this case, the final patch, is "1." or a  
> patch that,
> as I stated before, removes completely OGNL evaluation of user-entered
> values?
> I think that the conversation should follow two different  
> direction: one for
> the immediate future (to solve the vulnerability) and one for the  
> final
> decision.
> Antonio
> 2007/7/16, Ing. Andrea Vettori <>:
>> Antonio,
>> the recursion solve the problem because after the first "step" you
>> are exposing to the remote exploit. The first evaluation step is  
>> secure.
>> If you have %{foo} somewhere and you evaluate it if the property foo
>> does not contain an expression you are safe. If it contains an
>> expression you are recursive-evaluating and that's the problem.
>> You can't have a circular-reference (like the one that caused me to
>> find this big problem) if you disable recursion.
>> Moreover I think it is always a good coding practice when using
>> recursion to have a max loop limit to break off the loop if something
>> goes wrong.
>> It's the way the patch is implemented (I noticed my fault after
>> posting the patch) that prevents expression that is not recursive
>> like "%{foo} %{bar}" to be evaluated. I don't think it's a problem
>> but we should know that this type of expressions will not be
>> evaluated after installing the patch.
>> Maybe Don has addressed this kind of expression in his patch... I'll
>> take a look at it later.
>> Il giorno 16/lug/07, alle ore 16:09, Antonio Petrelli ha scritto:
>> > 2007/7/16, Ing. Andrea Vettori <>:
>> >>
>> >> What about expression like "%{foo} %{bar}" that work with the  
>> current
>> >> version but don't work using the loopCounter patch ?
>> >>
>> >> I don't think we need them but who knows...
>> >
>> >
>> >
>> > I think that recursion is a false problem: it's up to the  
>> developer to
>> > control it (I don't think that JSP EL controls it, correct me if  
>> I am
>> > wrong). Eventually a log message can be written, but preventing it
>> > is not a
>> > solution, because a particular case (such as circular reference)
>> > will be
>> > always present.
>> > The "real" problem is that a user should not be allowed to execute
>> > such OGNL
>> > code!
>> >
>> > Antonio
>> --
>> Ing. Andrea Vettori
>> Consulente per l'Information Technology
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail:
>> For additional commands, e-mail:

Ing. Andrea Vettori
Consulente per l'Information Technology

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message