Hello Markus (and Struts Developers List).

Thanks for confirming that changing the expressionMaxLength value to 1024 did 
actually suppress the exception behaviour and warning output you original 
received with the test build of 2.5.21. That suggestion was more to confirm 
that changing the value via configuration behaved as expected than it being a 
"fix" per se.

You raised some valid concerns and had a good discussion with Yasser with 
respect to expressionMaxLength and its usage implications for 2.5.x and 2.6. 
Thanks to both you and Yasser for that. :)

Taking into consideration that discussion and Łukasz' comments it seems some 
options for the Struts 2.5.21 "struts.ognl.expressionMaxLength" could be:

a) Disable it by default (commenting out the entry in default.properties should 
work ?).
b) Set it to Integer.MAX_VALUE (2147483647).
c) Set it to some power-of-two unlikely to produce errors for virtually any 
"reasonable" expression length (e.g. 16384).

For 2.5.21 maybe a) would be cleanest (as per Markus' comments), with 
individual applications still able to set a limit, should they wish to do so 
via configuration.

For 2.6 maybe a lower value for a default (such as Łukasz' suggestion of 256) 
would be OK, provided it is a clearly documented in 2.6.

James.
     On Friday, November 8, 2019, 1:14:06 a.m. EST, i...@flyingfischer.ch 
<i...@flyingfischer.ch> wrote:  
 
 Hello JC

thanks for replying. There are several flaws with the idea to limit the
length of a OGNL expression string due to secutity reasons:

First: the parsing of the expression will be BLOCKED, as intended, and
an exception is being thrown:

ognl.OgnlException: Parsing blocked due to security reasons!
[java.lang.SecurityException: This expression exceeded maximum allowed
length:

However, it is being logged as WARNING:

WARN com.opensymphony.xwork2.ognl.OgnlUtil - Could not evaluate this
expression due to security constraints

So first off, if using such a measure, log it as what it is: an ERROR.

Second, could you please elaborate what the LENGTH of a OGNL expression
has to do with security? Do you mean, just because the bad guys tend to
use lengthy expressions to hack a struts application it is a good idea
to limit the length of a OGNL expression as such?

If we can see that car accidents tend to happen with heavy cars driven
by drunken people cause the most damage, should we then restrict the
damage by limiting the weight of a car? I do not think this is a
sensible approach.

Limiting OGNL expressions is hiding a real problem by obfuscating the
real problem by something that is not affiliated with the root cause.
This may hinder the early detection of any true root cause to take
appropriate counter measures and is therefore anything else than a
security measure.

in contrary: you may cause true security issues when application start
to have lacking functionality due to "parsing errors" by limiting OGNL
expression.

Any limit will anyway always be totally arbitrarily: Assuming a
limitation of 30 characters and expression like this will pass <s:if
test="a.b.size() > 0 || c.d.size() > 0"> while <s:if
test="firstObject.myCollection() > 0 ||
secondObject.myCollection().size() > 0"> will fail. Raising the
limitation to 200 characters or 1024 or whatever does not change this at
all.

So if you are still convinced that implementing such a restriction is a
good idea, the default setting in Struts/OGNL should be that such a
limitation is disabled by default and may be enabled at best on
recommendation.

Looking at your "fix":

<constant name="struts.ognl.expressionMaxLength" value="1024" />

This does "fix" the warnings and errors in my case of course. However
there seems not to be any possibility to DISABLE this limitation
totally: <constant name="struts.ognl.expressionMaxLength" value="0" />
yields any expression as non functional.

Thanks for reconsidering this weak design decision in Struts!

Markus

PS: we had a discussion on this matter before on this list, and there
was no consensus to implement it.


Am 08.11.19 um 02:07 schrieb J C:
>  Sorry - theree is a typo I missed in copy/paste. That should have been:
>
> (if using struts.xml) -
> <constant name="struts.ognl.expressionMaxLength" value="1024" />
>
> James.    On Thursday, November 7, 2019, 8:02:13 p.m. EST, J C 
> <jcyh24...@yahoo.ca.invalid> wrote:  
>  
>  (Sorry about the separate thread for reply)
>
> Hello Markus.
>
> If you have expressions in your application longer than the default limit in 
> 2.5.21 (200), that may be causing the exception (and hopefully also the WARN 
> output).
>
> Please try applying a configuration change for your application (replace 1024 
> with whatever is the largest expression length you need for your application):
>
> (if using struts.properties) -
> struts.ognl.expressionMaxLength=1024
>
> (if using struts.xml) -
> <constant name="sstruts.ognl.expressionMaxLength" value="1024" />
>
> and see if that resolves the failure ?
>
> Please reply to the dev list to let us know if that helps or not.
>
> Thanks,
>
> James.
>
>> It is reported in WARN level:
>>
>> WARN com.opensymphony.xwork2.ognl.OgnlValueStack - Could not evaluate
>> this expression due to security constraints:
>>
>> Markus
>>
>>> Am 07.11.19 um 23:12 schrieb i...@flyingfischer.ch:
>>> See new errors like this:
>>>
>>> Caused by: java.lang.SecurityException: This expression exceeded maximum
>>> allowed length:..
>>>
>>> followed by a longer OGNL expression in JSP.
>>>
>>> Markus
>>>
>>>> Am 07.11.19 um 20:57 schrieb Lukasz Lenart:
>>>> Hi,
>>>>
>>>> Please take a time and test the bits - any help is appreciated. Please
>>>> report any problems. I'll call for a vote in a few days if no problems
>>>> will be spotted.
>>>>
>>>> Staging Maven repo
>>>> https://repository.apache.org/content/groups/staging/
>>>> Standalone artifacts
>>>> https://dist.apache.org/repos/dist/dev/struts/2.5.21/
>>>>
>>>> Release notes
>>>> https://cwiki.apache.org/confluence/display/WW/Version+Notes+2.5.21
>>>>
>>>>
>>>> Kind regards
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
>>>> For additional commands, e-mail: dev-h...@struts.apache.org
>>>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org
> For additional commands, e-mail: dev-h...@struts.apache.org
>
>  


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

  

Reply via email to