Dear All I am perfectly with Yassers proposition. Thank you all for this good discussion and the willingness to revise taken decisions.
I appreciate your continuous and great work on struts since many years! Thanks a lot to all people involved. Best regards Markus Am 09.11.19 um 14:29 schrieb Yasser Zamani: > 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@struts.apache.org For additional commands, e-mail: dev-h...@struts.apache.org
