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
