struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "info@flyingfischer.ch" <i...@flyingfischer.ch>
Subject Re: Struts 2.5.21 test build is ready
Date Fri, 08 Nov 2019 06:13:52 GMT
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 <jcyh24768@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 info@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-unsubscribe@struts.apache.org
>>>> For additional commands, e-mail: dev-help@struts.apache.org
>>>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>   


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


Mime
View raw message