Hi JC,

Right now I concede that I think Markus is right. According to my vision on 
security reports, setting that to a number 200-400 gradually decimates any 
security benefit. According to his app, setting it to a lower value will likely 
bother users. So he correctly said: "I suspect it will never be possible with 
such an approach to find a general correct balance between the definition of a 
magically allowed number of characters and a real security benefit". He is also 
right in his rest of previous email.

Consequently, like him, I think this should be disabled by default in both 2.5 
and 2.6. Users can activate and set their limit according to their application 
profile if they were interested -- however, we should keep unit tests to ensure 
this works when users apply that config.

Regards.

>-----Original Message-----
>From: J C <jcyh24...@yahoo.ca.INVALID>
>Sent: Saturday, November 9, 2019 8:13 AM
>To: Struts Developers List <dev@struts.apache.org>
>Subject: Re: Struts 2.5.21 test build is ready
>
> 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
>
>

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

Reply via email to