Did you fix the "Memer" spelling error I noted on the review?
--- On Wed, 8/13/08, Musachy Barroso <[EMAIL PROTECTED]> wrote: > From: Musachy Barroso <[EMAIL PROTECTED]> > Subject: Re: ParameterFilterInterceptor security issue > To: "Struts Developers List" <[email protected]> > Date: Wednesday, August 13, 2008, 10:41 AM > the patch for WW-2761 was committed to xwork trunk > > On 8/12/08, Musachy Barroso <[EMAIL PROTECTED]> > 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 > <[EMAIL PROTECTED]> 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 > <[EMAIL PROTECTED]> > >> 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:[EMAIL PROTECTED] > >>> 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 <[EMAIL PROTECTED]> > >>> 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 <[EMAIL PROTECTED]> > >>>> 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: > [EMAIL PROTECTED] For > >>>>> additional commands, e-mail: > [EMAIL PROTECTED] > >>>>> > >>>>> > >>>> > >>>> > >>>> > >>>> -- > >>>> "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: > [EMAIL PROTECTED] For additional > >>> commands, e-mail: [EMAIL PROTECTED] > >>> > >>> > ---------------------------------------------------------------------- > >>> 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: > [EMAIL PROTECTED] > >>> For additional commands, e-mail: > [EMAIL PROTECTED] > >>> > >>> > >> > >> > >> > >> -- > >> "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: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
