Re: Max length for OGNL expression

2019-09-17 Thread Lukasz Lenart
Hi,

I think by default OGNL can have this limit set to Integer.MAX_VALUE
or something like that, and we just must expose that setting to the
Struts users by a constant. BTW. there are few other things that
should be exposed as well e.g.: debugging OGNL expression


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

pon., 16 wrz 2019 o 15:58 i...@flyingfischer.ch
 napisał(a):
>
> Dear Yasser
>
> I perfectly understood that the proposed change is proactive and that
> there are no open known vulnerabilities. ;-)
>
> Best regards
> Markus
>
> Am 16.09.19 um 15:42 schrieb Yasser Zamani:
> >> -Original Message-
> >> From: i...@flyingfischer.ch 
> >> Sent: Monday, September 16, 2019 4:58 PM
> >> To: dev@struts.apache.org
> >> Subject: Re: Max length for OGNL expression
> >>
> >> Dear Yasser
> >>
> >> we definitively need an option to totally disable this "feature". It 
> >> really depends
> >> on what kind of application you deploy.
> > Yes it's going to be user configurable and setting it to 0 would disable it 
> > :)
> >
> >> Logging a warning seems appropriate. But we should avoid logging a warning
> >> while the "feature" is disabled.
> > Yes of course, +1.
> >
> >> I also fear that this will lead to vulnerable applications, because the 
> >> "default"
> >> settings are not vulnerable to a given attack and therefore the framework 
> >> "needs
> >> no fixing". As I said before: the proposed fix is at the wrong place. I 
> >> may be
> >> mistaken, but we should try to find solutions at the core of the problem.
> > (Sorry I forgot to mention earlier) Yes we have already fixed all known 
> > vulnerabilities. This one is just a proactive try for possible unknown 
> > expression injection vulnerabilities which are discovered by hacker sooner 
> > than us. So this is going to make it harder to actually exploit such 
> > unknown vulnerabilities.
> >
> > Kind Regards.
> >
> >> However, as long as we have an option to disable this, it should work out.
> >>
> >> Markus
> >>
> >>
> >> Am 16.09.19 um 14:09 schrieb Yasser Zamani:
> >>> Thanks Markus and Christoph! Please see inline and see if it satisfies 
> >>> those
> >> challenges.
> >>>> -Original Message-
> >>>> From: christoph.nenn...@bmw.de 
> >>>> Sent: Monday, September 16, 2019 11:39 AM
> >>>> To: dev@struts.apache.org
> >>>> Subject: AW: Max length for OGNL expression
> >>>>
> >>>> I agree with this. Basically I like the idea to limit length of ognl
> >>>> and I think it would increase security. But IMHO it is likely to
> >>>> cause issues in applications and thus applications must be able to 
> >>>> control it.
> >>> Yes. By "config element" I meant they will be able to override it or set 
> >>> it to a
> >> very large number to disable it in their struts.xml. Please see also below.
> >>>> Regards,
> >>>> Christoph
> >>>>
> >>>>
> >>>>> Seems to me not to be the right place to correct any possible
> >>>>> problems, and far off any related root of a possible issue.
> >>>>>
> >>>>> The config would definitively need an option to be disabled totally.
> >>>>> I expect very unexpected and hard to trace side effects, depending
> >>>>> on the application in place.
> >>> Yes I have already thought that users might have long expressions e.g. 
> >>>  >> test="" but I think there exists an N such that no user 
> >> has any
> >> expression longer than N. For example I guess N=200. Somebody might claim
> >> N=500 but at bottom we can discuss and find a default for N.
> >>> Furthermore we will log.warn() such unlikely expressions before
> >>> blocking them, so user will be able to find and amend them which makes
> >>> code more readable and maintainable -- obviously a raw expression
> >>> longer than 200 should be hard to maintain and read :)
> >>>
> >>> Best Regards.
> >>>
> >>>>> Markus
> >>>>>
> >>>>> Am 15.09.19 um 09:58 schrieb Yasser Zamani:
> >>>>>> Hi,
> >>>>>>
> >>>>>> I thought it might be nice to add a config 

Re: Max length for OGNL expression

2019-09-16 Thread i...@flyingfischer.ch
Dear Yasser

I perfectly understood that the proposed change is proactive and that
there are no open known vulnerabilities. ;-)

Best regards
Markus

Am 16.09.19 um 15:42 schrieb Yasser Zamani:
>> -Original Message-
>> From: i...@flyingfischer.ch 
>> Sent: Monday, September 16, 2019 4:58 PM
>> To: dev@struts.apache.org
>> Subject: Re: Max length for OGNL expression
>>
>> Dear Yasser
>>
>> we definitively need an option to totally disable this "feature". It really 
>> depends
>> on what kind of application you deploy.
> Yes it's going to be user configurable and setting it to 0 would disable it :)
>
>> Logging a warning seems appropriate. But we should avoid logging a warning
>> while the "feature" is disabled.
> Yes of course, +1.
>
>> I also fear that this will lead to vulnerable applications, because the 
>> "default"
>> settings are not vulnerable to a given attack and therefore the framework 
>> "needs
>> no fixing". As I said before: the proposed fix is at the wrong place. I may 
>> be
>> mistaken, but we should try to find solutions at the core of the problem.
> (Sorry I forgot to mention earlier) Yes we have already fixed all known 
> vulnerabilities. This one is just a proactive try for possible unknown 
> expression injection vulnerabilities which are discovered by hacker sooner 
> than us. So this is going to make it harder to actually exploit such unknown 
> vulnerabilities.
>
> Kind Regards.
>
>> However, as long as we have an option to disable this, it should work out.
>>
>> Markus
>>
>>
>> Am 16.09.19 um 14:09 schrieb Yasser Zamani:
>>> Thanks Markus and Christoph! Please see inline and see if it satisfies those
>> challenges.
>>>> -Original Message-
>>>> From: christoph.nenn...@bmw.de 
>>>> Sent: Monday, September 16, 2019 11:39 AM
>>>> To: dev@struts.apache.org
>>>> Subject: AW: Max length for OGNL expression
>>>>
>>>> I agree with this. Basically I like the idea to limit length of ognl
>>>> and I think it would increase security. But IMHO it is likely to
>>>> cause issues in applications and thus applications must be able to control 
>>>> it.
>>> Yes. By "config element" I meant they will be able to override it or set it 
>>> to a
>> very large number to disable it in their struts.xml. Please see also below.
>>>> Regards,
>>>> Christoph
>>>>
>>>>
>>>>> Seems to me not to be the right place to correct any possible
>>>>> problems, and far off any related root of a possible issue.
>>>>>
>>>>> The config would definitively need an option to be disabled totally.
>>>>> I expect very unexpected and hard to trace side effects, depending
>>>>> on the application in place.
>>> Yes I have already thought that users might have long expressions e.g. > test="" but I think there exists an N such that no user has 
>> any
>> expression longer than N. For example I guess N=200. Somebody might claim
>> N=500 but at bottom we can discuss and find a default for N.
>>> Furthermore we will log.warn() such unlikely expressions before
>>> blocking them, so user will be able to find and amend them which makes
>>> code more readable and maintainable -- obviously a raw expression
>>> longer than 200 should be hard to maintain and read :)
>>>
>>> Best Regards.
>>>
>>>>> Markus
>>>>>
>>>>> Am 15.09.19 um 09:58 schrieb Yasser Zamani:
>>>>>> Hi,
>>>>>>
>>>>>> I thought it might be nice to add a config element which confines
>>>>>> the length of OGNL expression that Struts is going to evaluate. It
>>>>>> is going to make hackers life harder :)
>>>>>>
>>>>>> How do you see it?
>>>>>>
>>>>>> Best.
>>>>>>
>>>>>>
>>>>>> ---
>>>>>> -
>>>>>> - 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
>


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



RE: Max length for OGNL expression

2019-09-16 Thread Yasser Zamani



>-Original Message-
>From: i...@flyingfischer.ch 
>Sent: Monday, September 16, 2019 4:58 PM
>To: dev@struts.apache.org
>Subject: Re: Max length for OGNL expression
>
>Dear Yasser
>
>we definitively need an option to totally disable this "feature". It really 
>depends
>on what kind of application you deploy.

Yes it's going to be user configurable and setting it to 0 would disable it :)

>
>Logging a warning seems appropriate. But we should avoid logging a warning
>while the "feature" is disabled.

Yes of course, +1.

>
>I also fear that this will lead to vulnerable applications, because the 
>"default"
>settings are not vulnerable to a given attack and therefore the framework 
>"needs
>no fixing". As I said before: the proposed fix is at the wrong place. I may be
>mistaken, but we should try to find solutions at the core of the problem.

(Sorry I forgot to mention earlier) Yes we have already fixed all known 
vulnerabilities. This one is just a proactive try for possible unknown 
expression injection vulnerabilities which are discovered by hacker sooner than 
us. So this is going to make it harder to actually exploit such unknown 
vulnerabilities.

Kind Regards.

>
>However, as long as we have an option to disable this, it should work out.
>
>Markus
>
>
>Am 16.09.19 um 14:09 schrieb Yasser Zamani:
>> Thanks Markus and Christoph! Please see inline and see if it satisfies those
>challenges.
>>
>>> -Original Message-----
>>> From: christoph.nenn...@bmw.de 
>>> Sent: Monday, September 16, 2019 11:39 AM
>>> To: dev@struts.apache.org
>>> Subject: AW: Max length for OGNL expression
>>>
>>> I agree with this. Basically I like the idea to limit length of ognl
>>> and I think it would increase security. But IMHO it is likely to
>>> cause issues in applications and thus applications must be able to control 
>>> it.
>> Yes. By "config element" I meant they will be able to override it or set it 
>> to a
>very large number to disable it in their struts.xml. Please see also below.
>>
>>> Regards,
>>> Christoph
>>>
>>>
>>>> Seems to me not to be the right place to correct any possible
>>>> problems, and far off any related root of a possible issue.
>>>>
>>>> The config would definitively need an option to be disabled totally.
>>>> I expect very unexpected and hard to trace side effects, depending
>>>> on the application in place.
>> Yes I have already thought that users might have long expressions e.g. test="" but I think there exists an N such that no user has 
>any
>expression longer than N. For example I guess N=200. Somebody might claim
>N=500 but at bottom we can discuss and find a default for N.
>>
>> Furthermore we will log.warn() such unlikely expressions before
>> blocking them, so user will be able to find and amend them which makes
>> code more readable and maintainable -- obviously a raw expression
>> longer than 200 should be hard to maintain and read :)
>>
>> Best Regards.
>>
>>>> Markus
>>>>
>>>> Am 15.09.19 um 09:58 schrieb Yasser Zamani:
>>>>> Hi,
>>>>>
>>>>> I thought it might be nice to add a config element which confines
>>>>> the length of OGNL expression that Struts is going to evaluate. It
>>>>> is going to make hackers life harder :)
>>>>>
>>>>> How do you see it?
>>>>>
>>>>> Best.
>>>>>
>>>>>
>>>>> ---
>>>>> -
>>>>> - 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



Re: Max length for OGNL expression

2019-09-16 Thread i...@flyingfischer.ch
Dear Yasser

we definitively need an option to totally disable this "feature". It
really depends on what kind of application you deploy.

Logging a warning seems appropriate. But we should avoid logging a
warning while the "feature" is disabled.

I also fear that this will lead to vulnerable applications, because the
"default" settings are not vulnerable to a given attack and therefore
the framework "needs no fixing". As I said before: the proposed fix is
at the wrong place. I may be mistaken, but we should try to find
solutions at the core of the problem.

However, as long as we have an option to disable this, it should work out.

Markus


Am 16.09.19 um 14:09 schrieb Yasser Zamani:
> Thanks Markus and Christoph! Please see inline and see if it satisfies those 
> challenges.
>
>> -Original Message-
>> From: christoph.nenn...@bmw.de 
>> Sent: Monday, September 16, 2019 11:39 AM
>> To: dev@struts.apache.org
>> Subject: AW: Max length for OGNL expression
>>
>> I agree with this. Basically I like the idea to limit length of ognl and I 
>> think it would
>> increase security. But IMHO it is likely to cause issues in applications and 
>> thus
>> applications must be able to control it.
> Yes. By "config element" I meant they will be able to override it or set it 
> to a very large number to disable it in their struts.xml. Please see also 
> below.
>
>> Regards,
>> Christoph
>>
>>
>>> Seems to me not to be the right place to correct any possible
>>> problems, and far off any related root of a possible issue.
>>>
>>> The config would definitively need an option to be disabled totally. I
>>> expect very unexpected and hard to trace side effects, depending on
>>> the application in place.
> Yes I have already thought that users might have long expressions e.g.  test="" but I think there exists an N such that no user has 
> any expression longer than N. For example I guess N=200. Somebody might claim 
> N=500 but at bottom we can discuss and find a default for N.
>
> Furthermore we will log.warn() such unlikely expressions before blocking 
> them, so user will be able to find and amend them which makes code more 
> readable and maintainable -- obviously a raw expression longer than 200 
> should be hard to maintain and read :)
>
> Best Regards.
>
>>> Markus
>>>
>>> Am 15.09.19 um 09:58 schrieb Yasser Zamani:
>>>> Hi,
>>>>
>>>> I thought it might be nice to add a config element which confines
>>>> the length of OGNL expression that Struts is going to evaluate. It
>>>> is going to make hackers life harder :)
>>>>
>>>> How do you see it?
>>>>
>>>> Best.
>>>>
>>>>
>>>> 
>>>> - 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



RE: Max length for OGNL expression

2019-09-16 Thread Yasser Zamani
Thanks Markus and Christoph! Please see inline and see if it satisfies those 
challenges.

>-Original Message-
>From: christoph.nenn...@bmw.de 
>Sent: Monday, September 16, 2019 11:39 AM
>To: dev@struts.apache.org
>Subject: AW: Max length for OGNL expression
>
>I agree with this. Basically I like the idea to limit length of ognl and I 
>think it would
>increase security. But IMHO it is likely to cause issues in applications and 
>thus
>applications must be able to control it.

Yes. By "config element" I meant they will be able to override it or set it to 
a very large number to disable it in their struts.xml. Please see also below.

>
>Regards,
>Christoph
>
>
>> Seems to me not to be the right place to correct any possible
>> problems, and far off any related root of a possible issue.
>>
>> The config would definitively need an option to be disabled totally. I
>> expect very unexpected and hard to trace side effects, depending on
>> the application in place.

Yes I have already thought that users might have long expressions e.g. >
>> Markus
>>
>> Am 15.09.19 um 09:58 schrieb Yasser Zamani:
>> > Hi,
>> >
>> > I thought it might be nice to add a config element which confines
>> > the length of OGNL expression that Struts is going to evaluate. It
>> > is going to make hackers life harder :)
>> >
>> > How do you see it?
>> >
>> > Best.
>> >
>> >
>> > 
>> > - 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



AW: Max length for OGNL expression

2019-09-16 Thread Christoph.Nenning
I agree with this. Basically I like the idea to limit length of ognl and I 
think it would increase security. But IMHO it is likely to cause issues in 
applications and thus applications must be able to control it.

Regards,
Christoph


> Seems to me not to be the right place to correct any possible problems,
> and far off any related root of a possible issue.
> 
> The config would definitively need an option to be disabled totally. I
> expect very unexpected and hard to trace side effects, depending on the
> application in place.
> 
> Markus
> 
> Am 15.09.19 um 09:58 schrieb Yasser Zamani:
> > Hi,
> >
> > I thought it might be nice to add a config element which confines the length
> > of OGNL expression that Struts is going to evaluate. It is going to make
> > hackers life harder :)
> >
> > How do you see it?
> >
> > Best.
> >
> >
> > -
> > 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


Re: Max length for OGNL expression

2019-09-15 Thread i...@flyingfischer.ch
Seems to me not to be the right place to correct any possible problems,
and far off any related root of a possible issue.

The config would definitively need an option to be disabled totally. I
expect very unexpected and hard to trace side effects, depending on the
application in place.

Markus

Am 15.09.19 um 09:58 schrieb Yasser Zamani:
> Hi,
>
> I thought it might be nice to add a config element which confines the length
> of OGNL expression that Struts is going to evaluate. It is going to make
> hackers life harder :)
>
> How do you see it?
>
> Best.
>
>
> -
> 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



Max length for OGNL expression

2019-09-15 Thread Yasser Zamani
Hi,

I thought it might be nice to add a config element which confines the length
of OGNL expression that Struts is going to evaluate. It is going to make
hackers life harder :)

How do you see it?

Best.


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