struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Musachy Barroso" <musa...@gmail.com>
Subject Re: ParameterFilterInterceptor security issue
Date Wed, 13 Aug 2008 14:41:08 GMT
the patch for WW-2761 was committed to xwork trunk

On 8/12/08, Musachy Barroso <musachy@gmail.com> wrote:
> Jira ticket with patch: https://issues.apache.org/struts/browse/WW-2761
> Crucible review: http://fisheye6.atlassian.com/cru/CR-10/preview
>
> I added this tests to the testcases:
>
>  put("blah", "This is blah"); //assert it is good
>  put("name", "try_1");  //assert it is not set
>  put("(name)", "try_2"); //assert it is not set
>  put("['name']", "try_3"); //assert it is not set
>
> if you can think of other tests, suggest them to add them. Making this
> patch I found a bug in parameter interceptor which I had to fix:
>
> protected boolean isAccepted(String paramName) {
>         if (!this.acceptedParams.isEmpty()) {
>             for (Pattern pattern : acceptedParams) {
>                 Matcher matcher = pattern.matcher(paramName);
>                 if (matcher.matches()) {
>                     return true; // <-- was returning false here
>                 }
>             }
>         }
>         return acceptedPattern.matcher(paramName).matches();
>     }
>
> please review it and let me know. I will commit it later, and will
> also look into Brian Relph's patch (WW-2760 )
>
> musachy
>
>
> On Tue, Aug 12, 2008 at 10:27 AM, Musachy Barroso <musachy@gmail.com> wrote:
>> I think that would be great, create a jira ticket, attach a patch to
>> it and we will check it out. I am writing a patch for the approach
>> that I mentioned before, which will provide the same behavior using
>> xml configuration, but having annotations as an option would be good.
>>
>> musachy
>>
>> On Tue, Aug 12, 2008 at 10:22 AM, Relph,Brian <Brian.Relph@cerner.com>
>> wrote:
>>>
>>> I wrote an annotation based parameters interceptor that extends the
>>> current parameters interceptor while allowing you to configure the
>>> default "accept" policy for an actions properties, as well as a
>>> per-property annotation that can override the action's policy.  This lets
>>> you use the same interceptor / interceptor stack for all actions, and
>>> configure each individually to accept or reject parameters.  I would
>>> still like to add some regex support to the action annotation.  Would
>>> this interest you?
>>>
>>> Brian Relph
>>>
>>> -----Original Message-----
>>> From: Musachy Barroso [mailto:musachy@gmail.com]
>>> Sent: Tuesday, August 12, 2008 8:53 AM
>>> To: Struts Developers List
>>> Subject: Re: ParameterFilterInterceptor security issue
>>>
>>> I forgot to say, that this would prevent all the OGNL expression tricks,
>>> because the property name that is passed to MemberAccess to be checked,
>>> is the actual property name, and not an expression.
>>>
>>> musachy
>>>
>>> On Tue, Aug 12, 2008 at 9:48 AM, Musachy Barroso <musachy@gmail.com>
>>> wrote:
>>>> It seems to me like there is an elegant solution to this. We can
>>>> rename StaticMemeberAccess to SecurityMemeberAccess, and in there not
>>>> only block static member access, but also fields that can be
>>>> configured using regular expressions. The params interceptor would
>>>> just set these fields before binding the params.
>>>>
>>>> //OGNL parameter binding just went to #2 in my "must kill" list :)
>>>>
>>>> musachy
>>>>
>>>> On Tue, Aug 12, 2008 at 9:14 AM, Rene Gielen <gielen@it-neering.net>
>>>> wrote:
>>>>>
>>>>> Am Di, 12.08.2008, 14:20, schrieb Jeromy Evans:
>>>>>>
>>>>>> This relates to Musachy's recent proposal to remove OGNL entirely
>>>>>> from the parameter-setting process.  Which I think is a very good
>>>>>> idea.
>>>>>>
>>>>>
>>>>> Indeed removing OGNL for parameters would fix this issue, but even if
>>>>> we would decide to do so this won't be trivial and might have many
>>>>> side effects.
>>>>>
>>>>>> If I've understood correctly, currently there is no way to filter
>>>>>> the parameter names, using regex or otherwise, other than to verify
>>>>>> them use a whitelist of valid names.
>>>>>>
>>>>>
>>>>> You can blacklist parameter names in the ParameterInterceptor ref,
>>>>> including the possibility to define RegExp patterns. The latter one
>>>>> is not possible for the ParameterFilterInterceptor right now, which I
>>>>> think is a feature we should add.
>>>>>
>>>>> Jelmer, would you mind creating an Jira issue for that?
>>>>> https://issues.apache.org/struts/
>>>>>
>>>>> - Rene
>>>>>
>>>>>> jelmer wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> I was looking into an easy way to prevent people binding on fields
>>>>>>> they shouldn't be binding on.
>>>>>>>
>>>>>>> Say you have a User object, you do not want people to be able
to
>>>>>>> bind on the isAdmin property.
>>>>>>>
>>>>>>> Various people remommended using the ParameterFilterInterceptor
for
>>>>>>> this but it seems to be flatout broken
>>>>>>>
>>>>>>> When you configure an action like this
>>>>>>>
>>>>>>> <action name="test" class="com.webapp.action.TestAction">
>>>>>>>     <interceptor-ref name="param-namevalue-filter">
>>>>>>>        <param name="blocked">name</param>
>>>>>>>     </interceptor-ref>
>>>>>>>     <interceptor-ref name="params"/> </action>
>>>>>>>
>>>>>>> then this wont work :
>>>>>>>
>>>>>>> /test.action?name=myname
>>>>>>>
>>>>>>> but this does :
>>>>>>>
>>>>>>> /test.action?(name)=jelmer
>>>>>>>
>>>>>>> and so does this
>>>>>>>
>>>>>>> /test.action?((name))=jelmer
>>>>>>>
>>>>>>> And so on, infact it is impossible to block any parameter
>>>>>>> effectively with the ParameterFilterInterceptor.
>>>>>>>
>>>>>>>
>>>>>>> Btw. I am aware that there is also the excludeParams method on
the
>>>>>>> ParametersInterceptor that accepts a regexp, so theoretically
you
>>>>>>> could use this to block parameters effectively but it would be
>>>>>>> extremely hard to write a correct regexp for it. Also I havent
>>>>>>> found a way to configure both params interceptors in a
>>>>>>> paramsPrepareParamsStack. This will only configure the first
params
>>>>>>> interceptor in the stack
>>>>>>>
>>>>>>> <interceptor-ref name="clientCrudStack">
>>>>>>>    <param name="params.excludeParams">some pattern</param>
>>>>>>> </interceptor-ref>
>>>>>>>
>>>>>>> Struts really seems to be lacking in this area.
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org For
>>>>> additional commands, e-mail: dev-help@struts.apache.org
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> "Hey you! Would you help me to carry the stone?" Pink Floyd
>>>>
>>>
>>>
>>>
>>> --
>>> "Hey you! Would you help me to carry the stone?" Pink Floyd
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org For additional
>>> commands, e-mail: dev-help@struts.apache.org
>>>
>>> ----------------------------------------------------------------------
>>> CONFIDENTIALITY NOTICE This message and any included attachments are from
>>> Cerner Corporation and are intended only for the addressee. The
>>> information contained in this message is confidential and may constitute
>>> inside or non-public information under international, federal, or state
>>> securities laws. Unauthorized forwarding, printing, copying,
>>> distribution, or use of such information is strictly prohibited and may
>>> be unlawful. If you are not the addressee, please promptly delete this
>>> message and notify the sender of the delivery error by e-mail or you may
>>> call Cerner's corporate offices in Kansas City, Missouri, U.S.A at (+1)
>>> (816)221-1024.
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
>>> For additional commands, e-mail: dev-help@struts.apache.org
>>>
>>>
>>
>>
>>
>> --
>> "Hey you! Would you help me to carry the stone?" Pink Floyd
>>
>
>
>
> --
> "Hey you! Would you help me to carry the stone?" Pink Floyd
>


-- 
"Hey you! Would you help me to carry the stone?" Pink Floyd

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


Mime
View raw message