Struts 2.5.19 test build is ready

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

Struts 2.5.19 test build is ready

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 week 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.19/

Release notes
https://cwiki.apache.org/confluence/display/WW/Version+Notes+2.5.19


Kind regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.19 test build is ready

Greg Huber
My dev logs are now full of these messages!

2019-01-04 08:29:33,861 WARN  com.opensymphony.xwork2.ognl.OgnlValueStack
OgnlValueStack:setDevMode - Setting development mode [true] affects the
safety of your application!

We already know this.

Cheers Greg

On Sun, 30 Dec 2018 at 16:05, Lukasz Lenart <[hidden email]> wrote:

> 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 week 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.19/
>
> Release notes
> https://cwiki.apache.org/confluence/display/WW/Version+Notes+2.5.19
>
>
> Kind regards
> --
> Łukasz
> + 48 606 323 122 http://www.lenart.org.pl/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
J C
Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.19 test build is ready

J C
In reply to this post by Lukasz Lenart
Hello Greg (and everyone).

Unfortunately it's a situation of trade-offs for detecting an unexpected change to devMode vs. when you have intentionally decided to do so.  Under the circumstances there may only be two ways to address the impact:

1) Suppress all log output below level Error for "com.opensymphony.xwork2.ognl.OgnlValueStack" using the application's log configuration (for loggers that support such).  If you're doing development with devMode true then it could be an appropriate response.

  If you're using Apache Commons Logging then suppressing the warnings should be possible with a commons-logging configuration entry something like:
  com.opensymphony.xwork2.ognl.OgnlValueStack.level=ERROR

  If you're using log4j2 then using a sample configuration from the Log4J site (https://logging.apache.org/log4j/2.x/manual/customloglevels.html#DefiningLevelsInConfiguration) as a reference, it should be possible to suppress the unwanted log messages using something like:
    <?xml version="1.0" encoding="UTF-8"?>
    <Configuration status="WARN">
       <Appenders>
          ... define your console/file appenders here ...
      </Appenders>
      <Loggers>
        <Logger name="com.opensymphony.xwork2.ognl.OgnlValueStack" level="ERROR" additivity="true">  <!-- Suppress WARN and below -->
        <Root level="trace">
          <AppenderRef ref="Console" level="debug" />
           ... define the rest of your appender reference here ...
        </Root>
      </Loggers>
    </Configuration>

2) Modify source com.opensymphony.xwork2.ognl.OgnlValueStack line 107 and change the log-level for the statement to "debug" in the Struts build.
  Doing that would avoid the output for logging configurations that use INFO and above levels, but developers would still see the message when running at debug level.

Trying 1) above with commons-logging worked for me, but unfortunately I don't have access to the configuration I used to double-check right now.  For Log4J2 there's a nice general overview ( https://www.journaldev.com/7128/log4j2-example-tutorial-configuration-levels-appenders ) that might be of use as well.

It's probably possible with SLF4J as well, but I don't have experience with the syntax.

Could you try to see if using a log configuration change something like the above works in your circumstances (to suppress unwanted warning output) and then let the Dev list know ?

Thanks,
James.

p.s.  I didn't encounter any functional issues with the 2.5.19 test build during some checks earlier in the week (mostly with core functionality and a little tiles-plugin).


On 2019/01/04 08:32:39, Greg Huber <[hidden email]> wrote:

> My dev logs are now full of these messages!>
>
> 2019-01-04 08:29:33,861 WARN com.opensymphony.xwork2.ognl.OgnlValueStack>
> OgnlValueStack:setDevMode - Setting development mode [true] affects the>
> safety of your application!>
>
> We already know this.>
>
> Cheers Greg>
>
> On Sun, 30 Dec 2018 at 16:05, Lukasz Lenart <[hidden email]> wrote:>
>
> > 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 week 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.19/>
> >>
> > Release notes>
> > https://cwiki.apache.org/confluence/display/WW/Version+Notes+2.5.19>
> >>
> >>
> > Kind regards>
> > -->
> > Łukasz>
> > + 48 606 323 122 http://www.lenart.org.pl/>
> >>
> > --------------------------------------------------------------------->
> > To unsubscribe, e-mail: [hidden email]>
> > For additional commands, e-mail: [hidden email]>
> >>
> >>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.19 test build is ready

Greg Huber
For me, I get four of these messages per request, so there are alot of
messages.  Also I would rather not change the log level.

In your build you should take care of these mistakes, and update your
config accordingly.

This is what I use in my pom, where I set all the dev options to false.

                            <execution>
                                <id>update-struts</id>
                                <phase>process-resources</phase>
                                <configuration>
                                    <target name="struts-update">
                                        <echo level="info">Update
struts.properties dev runtime to false</echo>
                                        <replace
dir="${project.build.directory}/classes">
                                            <include
name="struts.properties" />
                                            <replacefilter
token="struts.devMode=true" value="struts.devMode=false" />
                                            <replacefilter
token="struts.configuration.xml.reload=true"
value="struts.configuration.xml.reload=false" />
                                            <replacefilter
token="struts.i18n.reload=true" value="struts.i18n.reload=false" />
                                        </replace>
                                        <property prefix="check"
file="${project.build.directory}/classes/struts.properties" />
                                        <echo>

struts.devMode=${check.struts.devMode}
                                        </echo>
                                    </target>
                                </configuration>
                                <goals>
                                    <goal>run</goal>
                                </goals>
                            </execution>

Cheers Greg

On Fri, 4 Jan 2019 at 20:15, J C <[hidden email]> wrote:

> Hello Greg (and everyone).
>
> Unfortunately it's a situation of trade-offs for detecting an unexpected
> change to devMode vs. when you have intentionally decided to do so.  Under
> the circumstances there may only be two ways to address the impact:
>
> 1) Suppress all log output below level Error for
> "com.opensymphony.xwork2.ognl.OgnlValueStack" using the application's log
> configuration (for loggers that support such).  If you're doing development
> with devMode true then it could be an appropriate response.
>
>   If you're using Apache Commons Logging then suppressing the warnings
> should be possible with a commons-logging configuration entry something
> like:
>   com.opensymphony.xwork2.ognl.OgnlValueStack.level=ERROR
>
>   If you're using log4j2 then using a sample configuration from the Log4J
> site (
> https://logging.apache.org/log4j/2.x/manual/customloglevels.html#DefiningLevelsInConfiguration)
> as a reference, it should be possible to suppress the unwanted log messages
> using something like:
>     <?xml version="1.0" encoding="UTF-8"?>
>     <Configuration status="WARN">
>        <Appenders>
>           ... define your console/file appenders here ...
>       </Appenders>
>       <Loggers>
>         <Logger name="com.opensymphony.xwork2.ognl.OgnlValueStack"
> level="ERROR" additivity="true">  <!-- Suppress WARN and below -->
>         <Root level="trace">
>           <AppenderRef ref="Console" level="debug" />
>            ... define the rest of your appender reference here ...
>         </Root>
>       </Loggers>
>     </Configuration>
>
> 2) Modify source com.opensymphony.xwork2.ognl.OgnlValueStack line 107 and
> change the log-level for the statement to "debug" in the Struts build.
>   Doing that would avoid the output for logging configurations that use
> INFO and above levels, but developers would still see the message when
> running at debug level.
>
> Trying 1) above with commons-logging worked for me, but unfortunately I
> don't have access to the configuration I used to double-check right now.
> For Log4J2 there's a nice general overview (
> https://www.journaldev.com/7128/log4j2-example-tutorial-configuration-levels-appenders
> ) that might be of use as well.
>
> It's probably possible with SLF4J as well, but I don't have experience
> with the syntax.
>
> Could you try to see if using a log configuration change something like
> the above works in your circumstances (to suppress unwanted warning output)
> and then let the Dev list know ?
>
> Thanks,
> James.
>
> p.s.  I didn't encounter any functional issues with the 2.5.19 test build
> during some checks earlier in the week (mostly with core functionality and
> a little tiles-plugin).
>
>
> On 2019/01/04 08:32:39, Greg Huber <[hidden email]> wrote:
> > My dev logs are now full of these messages!>
> >
> > 2019-01-04 08:29:33,861 WARN com.opensymphony.xwork2.ognl.OgnlValueStack>
> > OgnlValueStack:setDevMode - Setting development mode [true] affects the>
> > safety of your application!>
> >
> > We already know this.>
> >
> > Cheers Greg>
> >
> > On Sun, 30 Dec 2018 at 16:05, Lukasz Lenart <[hidden email]> wrote:>
> >
> > > 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 week 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.19/>
> > >>
> > > Release notes>
> > > https://cwiki.apache.org/confluence/display/WW/Version+Notes+2.5.19>
> > >>
> > >>
> > > Kind regards>
> > > -->
> > > Łukasz>
> > > + 48 606 323 122 http://www.lenart.org.pl/>
> > >>
> > > --------------------------------------------------------------------->
> > > To unsubscribe, e-mail: [hidden email]>
> > > For additional commands, e-mail: [hidden email]>
> > >>
> > >>
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
J C
Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.19 test build is ready

J C
In reply to this post by Lukasz Lenart
Hello Greg (and Dev List).

Thanks for the reply and additional details of the concern you found with the 2.5.19 test build.

A PR request to address the OgnlValueStack "warn log flood" was opened and submitted for review.  If it gets accepted for 2.5.19, then it should help address the concern you identified.

We'll have to see what Łukasz and the Apache Struts Team decide for 2.5.19.

Thanks again,

James.


--------------------------------------------
On Sat, 1/5/19, Greg Huber <[hidden email]> wrote:

 Subject: Re: Struts 2.5.19 test build is ready
 To: "Struts Developers List" <[hidden email]>, "J C" <[hidden email]>
 Received: Saturday, January 5, 2019, 3:28 AM
 
 For me, I get four of these
 messages per request, so there are alot of messages.  Also
 I would rather not change the log level.
 In your build you should take care
 of these mistakes, and update your config
 accordingly.
 This is what I use in my pom, where
 I set all the dev options to false.
 
                    
         <execution>
    
                            
 <id>update-struts</id>
    
                            
 <phase>process-resources</phase>
                        
         <configuration>
    
                                
 <target name="struts-update">
                        
                 <echo
 level="info">Update struts.properties dev
 runtime to false</echo>
        
                                
 <replace
 dir="${project.build.directory}/classes">
                        
                     <include
 name="struts.properties" />
                        
                     <replacefilter
 token="struts.devMode=true"
 value="struts.devMode=false" />
                        
                     <replacefilter
 token="struts.configuration.xml.reload=true"
 value="struts.configuration.xml.reload=false"
 />
                    
                         <replacefilter
 token="struts.i18n.reload=true"
 value="struts.i18n.reload=false" />
                        
                 </replace>
                        
                 <property
 prefix="check"
 file="${project.build.directory}/classes/struts.properties"
 />
                    
                     <echo>
                        
                    
 struts.devMode=${check.struts.devMode}
                        
                 </echo>
                        
             </target>
    
                            
 </configuration>
            
                     <goals>
                        
             <goal>run</goal>
                        
         </goals>
        
                     </execution>
 
 Cheers Greg
 
 On Fri, 4
 Jan 2019 at 20:15, J C <[hidden email]>
 wrote:
 Hello Greg (and
 everyone).
 
 
 
 Unfortunately it's a situation of trade-offs for
 detecting an unexpected change to devMode vs. when you have
 intentionally decided to do so.  Under the circumstances
 there may only be two ways to address the impact:
 
 
 
 1) Suppress all log output below level Error for
 "com.opensymphony.xwork2.ognl.OgnlValueStack"
 using the application's log configuration (for loggers
 that support such).  If you're doing development with
 devMode true then it could be an appropriate response.
 
 
 
   If you're using Apache Commons Logging then
 suppressing the warnings should be possible with a
 commons-logging configuration entry something like:
 
  
 com.opensymphony.xwork2.ognl.OgnlValueStack.level=ERROR
 
 
 
   If you're using log4j2 then using a sample
 configuration from the Log4J site (https://logging.apache.org/log4j/2.x/manual/customloglevels.html#DefiningLevelsInConfiguration)
 as a reference, it should be possible to suppress the
 unwanted log messages using something like:
 
     <?xml version="1.0"
 encoding="UTF-8"?>
 
     <Configuration status="WARN">
 
        <Appenders>
 
           ... define your console/file appenders here
 ...
 
       </Appenders>
 
       <Loggers>
 
         <Logger
 name="com.opensymphony.xwork2.ognl.OgnlValueStack"
 level="ERROR" additivity="true"> 
 <!-- Suppress WARN and below -->
 
         <Root level="trace">
 
           <AppenderRef ref="Console"
 level="debug" />
 
            ... define the rest of your appender
 reference here ...
 
         </Root>
 
       </Loggers>
 
     </Configuration>
 
 
 
 2) Modify source com.opensymphony.xwork2.ognl.OgnlValueStack
 line 107 and change the log-level for the statement to
 "debug" in the Struts build.
 
   Doing that would avoid the output for logging
 configurations that use INFO and above levels, but
 developers would still see the message when running at debug
 level.
 
 
 
 Trying 1) above with commons-logging worked for me, but
 unfortunately I don't have access to the configuration I
 used to double-check right now.  For Log4J2 there's a
 nice general overview ( https://www.journaldev.com/7128/log4j2-example-tutorial-configuration-levels-appenders
 ) that might be of use as well.
 
 
 
 It's probably possible with SLF4J as well, but I
 don't have experience with the syntax.
 
 
 
 Could you try to see if using a log configuration change
 something like the above works in your circumstances (to
 suppress unwanted warning output) and then let the Dev list
 know ?
 
 
 
 Thanks,
 
 James.
 
 
 
 p.s.  I didn't encounter any functional issues with the
 2.5.19 test build during some checks earlier in the week
 (mostly with core functionality and a little
 tiles-plugin).
 
 
 
 
 
 On 2019/01/04 08:32:39, Greg Huber <[hidden email]>
 wrote:
 
 > My dev logs are now full of these messages!>
 
 >
 
 > 2019-01-04 08:29:33,861 WARN
 com.opensymphony.xwork2.ognl.OgnlValueStack>
 
 > OgnlValueStack:setDevMode - Setting development mode
 [true] affects the>
 
 > safety of your application!>
 
 >
 
 > We already know this.>
 
 >
 
 > Cheers Greg>
 
 >
 
 > On Sun, 30 Dec 2018 at 16:05, Lukasz Lenart <[hidden email]>
 wrote:>
 
 >
 
 > > 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
 week 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.19/>
 
 > >>
 
 > > Release notes>
 
 > > https://cwiki.apache.org/confluence/display/WW/Version+Notes+2.5.19>
 
 > >>
 
 > >>
 
 > > Kind regards>
 
 > > -->
 
 > > Łukasz>
 
 > > + 48 606 323 122 http://www.lenart.org.pl/>
 
 > >>
 
 > >
 --------------------------------------------------------------------->
 
 > > To unsubscribe, e-mail: [hidden email]>
 
 > > For additional commands, e-mail: [hidden email]>
 
 > >>
 
 > >>
 
 >
 
 
 
 ---------------------------------------------------------------------
 
 To unsubscribe, e-mail: [hidden email]
 
 For additional commands, e-mail: [hidden email]
 
 
 
 

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.19 test build is ready

Lukasz Lenart
sob., 5 sty 2019 o 22:46 J C <[hidden email]> napisał(a):
>
> Hello Greg (and Dev List).
>
> Thanks for the reply and additional details of the concern you found with the 2.5.19 test build.
>
> A PR request to address the OgnlValueStack "warn log flood" was opened and submitted for review.  If it gets accepted for 2.5.19, then it should help address the concern you identified.
>
> We'll have to see what Łukasz and the Apache Struts Team decide for 2.5.19.

After fixing this (and another issues with I18Interceptor), I will
drop 2.5.19 and then I will prepare a new test build - 2 5.20


Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.19 test build is ready

Lukasz Lenart
I will drop this test build and I'll prepare a new one.

pon., 7 sty 2019 o 09:27 Lukasz Lenart <[hidden email]> napisał(a):

>
> sob., 5 sty 2019 o 22:46 J C <[hidden email]> napisał(a):
> >
> > Hello Greg (and Dev List).
> >
> > Thanks for the reply and additional details of the concern you found with the 2.5.19 test build.
> >
> > A PR request to address the OgnlValueStack "warn log flood" was opened and submitted for review.  If it gets accepted for 2.5.19, then it should help address the concern you identified.
> >
> > We'll have to see what Łukasz and the Apache Struts Team decide for 2.5.19.
>
> After fixing this (and another issues with I18Interceptor), I will
> drop 2.5.19 and then I will prepare a new test build - 2 5.20
>
>
> Regards
> --
> Łukasz
> + 48 606 323 122 http://www.lenart.org.pl/

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

J C
Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

J C
In reply to this post by J C
(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 [hidden email]:
>> 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: [hidden email]
>>> For additional commands, e-mail: [hidden email]
>>>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

J C
Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

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 <[hidden email]> 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 [hidden email]:
>> 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: [hidden email]
>>> For additional commands, e-mail: [hidden email]
>>>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

 
Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

info@flyingfischer.ch
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 <[hidden email]> 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 [hidden email]:
>>> 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: [hidden email]
>>>> For additional commands, e-mail: [hidden email]
>>>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>  


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

Lukasz Lenart
In reply to this post by J C
pt., 8 lis 2019 o 02:02 J C <[hidden email]> napisał(a):

> 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.

If that will help I think we should increase the default and push a
new test build.


Regards
--
Łukasz
+ 48 606 323 122 http://www.lenart.org.pl/

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

Lukasz Lenart
Or maybe even use some very large number and reduce it to 256 in
Struts 2.6 :thinking:

pt., 8 lis 2019 o 08:02 Lukasz Lenart <[hidden email]> napisał(a):

>
> pt., 8 lis 2019 o 02:02 J C <[hidden email]> napisał(a):
> > 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.
>
> If that will help I think we should increase the default and push a
> new test build.
>
>
> Regards
> --
> Łukasz
> + 48 606 323 122 http://www.lenart.org.pl/

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

Yasser Zamani-2
In reply to this post by info@flyingfischer.ch
Hi Markus,

Sorry for inconvenience - yes that was my genius idea ;) ensued from my
vision on our security reports and in the first place, it didn't look
bad to me because I'd seen similar practices in variety of places for
example in http, tomcat, nginx and etc.

However, I also shared and discussed it in dev list and feedback was OK
and we only had doubt about the default constant value and if enable by
default; "However, as long as we have an option to disable this, it
should work out." you said. Setting it to an empty string will disable
it - sorry I couldn't find time but I'll document these and other new
functionalities ASAP.

You're right that "Limiting OGNL expressions is hiding a real problem"
(as it's just trying to be a proactive defense) and we shouldn't block
heavy cars only because of drunken people.

Maybe I'm wrong but the thing is that I didn't expect such long
expression in an application. Could you please count how many expression
fail at your side? (maybe they're few) Could you please share one of
them with us? (Does that one look rational? Is it easily readable and
maintainable? If not, isn't it better to move that logic into your
action and just use <s:if test="myLogic()" which makes it readable and
maintainable?).

Please see inline...

On 11/8/2019 9:43 AM, [hidden email] 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.

By WARN I had tried to warn user to fix such long expression via moving
it to action or etc. I did like Struts doesn't find a setter to set ognl
value. But now I concede you're right, ERROR is better. I'll pull
request to see :) thanks!

>
> 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.

Already discussed above. I need your answers to above questions please,
to rethink and analysis of this decision again. Thanks!

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

Sorry I couldn't get this. Could you please extend?

>
> 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.

I think there is always an N where it's not sensible to have an
expression longer than it, don't you?


>
> 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.

I agree. As you and Lukasz expressed, lets disable it in 2.5 and enable
it in 2.6 by default, thanks! (to confine future possible similar
inconveniences for users)

>
> 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" />

Setting it to an empty string will disable it - sorry I couldn't find
time but I'll document these and other new functionalities ASAP.

> yields any expression as non functional.
>
> Thanks for reconsidering this weak design decision in Struts!
>

Thank you so much for your time writing and analyzing to us! We really
appreciate it!

Kind Regards,
Yasser.

> 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 <[hidden email]> 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 [hidden email]:
>>>> 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: [hidden email]
>>>>> For additional commands, e-mail: [hidden email]
>>>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>    
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

info@flyingfischer.ch
Hi Yasser

thanks for reconsidering and your detailed answers. I appreciate your
detailed feedback very much. And thanks for specifying that there _is_
an option to disable the restrictions by using:

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

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. Therefore this setting
should be disabled by default and may optionally be activated. If
handled and declared in this manner, the option may prove helpful
indeed. And it should be documented, that this option only has proactive
character.

As you mention below, it is always possible to work around such
restrictions and to adapt any given logic, move OGNL expressions into
the program logic, shorten variable names and so on. However there are
valid use cases where the code remains easier to maintain by using a
longer OGNL expression (containing e.g. or statements), to avoid to
clutter every action with things that may belong into the view,
especially if you work in the view with central imports (e.g. for
headers, menus, footer etc.).

See also inline below...

Markus


Am 08.11.19 um 11:18 schrieb Yasser Zamani:

> Hi Markus,
>
> Sorry for inconvenience - yes that was my genius idea ;) ensued from my
> vision on our security reports and in the first place, it didn't look
> bad to me because I'd seen similar practices in variety of places for
> example in http, tomcat, nginx and etc.
>
> However, I also shared and discussed it in dev list and feedback was OK
> and we only had doubt about the default constant value and if enable by
> default; "However, as long as we have an option to disable this, it
> should work out." you said. Setting it to an empty string will disable
> it - sorry I couldn't find time but I'll document these and other new
> functionalities ASAP.
>
> You're right that "Limiting OGNL expressions is hiding a real problem"
> (as it's just trying to be a proactive defense) and we shouldn't block
> heavy cars only because of drunken people.
>
> Maybe I'm wrong but the thing is that I didn't expect such long
> expression in an application. Could you please count how many expression
> fail at your side? (maybe they're few) Could you please share one of
> them with us? (Does that one look rational? Is it easily readable and
> maintainable? If not, isn't it better to move that logic into your
> action and just use <s:if test="myLogic()" which makes it readable and
> maintainable?).
>
> Please see inline...
>
> On 11/8/2019 9:43 AM, [hidden email] 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.
> By WARN I had tried to warn user to fix such long expression via moving
> it to action or etc. I did like Struts doesn't find a setter to set ognl
> value. But now I concede you're right, ERROR is better. I'll pull
> request to see :) thanks!
>
>> 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.
> Already discussed above. I need your answers to above questions please,
> to rethink and analysis of this decision again. Thanks!
>
>> in contrary: you may cause true security issues when application start
>> to have lacking functionality due to "parsing errors" by limiting OGNL
>> expression.
> Sorry I couldn't get this. Could you please extend?

I may be wrong on this, but loosing information and decisions what
should be displayed or not based on a restricted OGNL expression could
pose real problems.

>> 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.
> I think there is always an N where it's not sensible to have an
> expression longer than it, don't you?

See above: I doubt that there is a sensible general approach, that
satisfies both application specific needs and a targeted security gain.
This may be well possible on individual level.

>
>
>> 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.
> I agree. As you and Lukasz expressed, lets disable it in 2.5 and enable
> it in 2.6 by default, thanks! (to confine future possible similar
> inconveniences for users)

Perfect. If handled like this, the option may prove useful.

>
>> 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" />
> Setting it to an empty string will disable it - sorry I couldn't find
> time but I'll document these and other new functionalities ASAP.

Thanks for this!

>
>> yields any expression as non functional.
>>
>> Thanks for reconsidering this weak design decision in Struts!
>>
> Thank you so much for your time writing and analyzing to us! We really
> appreciate it!
>
> Kind Regards,
> Yasser.
>
>> 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 <[hidden email]> 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 [hidden email]:
>>>>> 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: [hidden email]
>>>>>> For additional commands, e-mail: [hidden email]
>>>>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [hidden email]
>>> For additional commands, e-mail: [hidden email]
>>>
>>>    
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

J C
Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

J C
In reply to this post by info@flyingfischer.ch
 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, [hidden email] <[hidden email]> 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 <[hidden email]> 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 [hidden email]:
>>> 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: [hidden email]
>>>> For additional commands, e-mail: [hidden email]
>>>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

 
Reply | Threaded
Open this post in threaded view
|

RE: Struts 2.5.21 test build is ready

Yasser Zamani-2
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 <[hidden email]>
>Sent: Saturday, November 9, 2019 8:13 AM
>To: Struts Developers List <[hidden email]>
>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, [hidden email]
><[hidden email]> 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
>> <[hidden email]> 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 [hidden email]:
>>>> 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: [hidden email] For
>>>>> additional commands, e-mail: [hidden email]
>>>>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email] For
>> additional commands, e-mail: [hidden email]
>>
>>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: [hidden email] For additional
>commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

info@flyingfischer.ch
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 <[hidden email]>
>> Sent: Saturday, November 9, 2019 8:13 AM
>> To: Struts Developers List <[hidden email]>
>> 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, [hidden email]
>> <[hidden email]> 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
>>> <[hidden email]> 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 [hidden email]:
>>>>> 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: [hidden email] For
>>>>>> additional commands, e-mail: [hidden email]
>>>>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [hidden email] For
>>> additional commands, e-mail: [hidden email]
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email] For additional
>> commands, e-mail: [hidden email]
>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

J C
Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

J C
In reply to this post by info@flyingfischer.ch
Hello.

Did some testing of the showcase and rest-showcase applications in the 2.5.21 test build (and a very quick test of the 2.5.22 test build as well).  Things seemed to work properly in both cases with no obvious errors seen in the console or via browser navigation.  The tests also included modifying one of the JSPs in the 2.5.22 showcase application to use a really long expression and it parsed with no errors.

The showcase and rest-showcase were run under JDK 8 and JDK 11 using Tomcat 8.5 and Firefox, in case someone else has different results.

Thanks,

James.

p.s.  FYI there are a few 2.5.21 showcase items that don't work (blank/truncated page) but I ** confirmed the same behaviour using 2.5.20 showcase and 2.5.18 showcase ** so this is a long-standing behaviour with the showcase app (not introduced with 2.5.21 or 2.5.22).  The "blank" pages results were seen with: "Person Manager" (both links), as well as "CRUD", "Execute and wait" and "Token" under Examples.


> 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: [hidden email]
> For additional commands, e-mail: [hidden email]

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Struts 2.5.21 test build is ready

info@flyingfischer.ch
Hello

I am running 2.5.21 in production in several projects. Everything is
running fine and smooth. No issues so far.

Markus

Am 18.11.19 um 02:30 schrieb J C:

> Hello.
>
> Did some testing of the showcase and rest-showcase applications in the 2.5.21 test build (and a very quick test of the 2.5.22 test build as well).  Things seemed to work properly in both cases with no obvious errors seen in the console or via browser navigation.  The tests also included modifying one of the JSPs in the 2.5.22 showcase application to use a really long expression and it parsed with no errors.
>
> The showcase and rest-showcase were run under JDK 8 and JDK 11 using Tomcat 8.5 and Firefox, in case someone else has different results.
>
> Thanks,
>
> James.
>
> p.s.  FYI there are a few 2.5.21 showcase items that don't work (blank/truncated page) but I ** confirmed the same behaviour using 2.5.20 showcase and 2.5.18 showcase ** so this is a long-standing behaviour with the showcase app (not introduced with 2.5.21 or 2.5.22).  The "blank" pages results were seen with: "Person Manager" (both links), as well as "CRUD", "Execute and wait" and "Token" under Examples.
>
>
>> 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: [hidden email]
>> For additional commands, e-mail: [hidden email]
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]