agh..never saw that, although I removed that method because it was never used. I hadn't noticed the difference between "excludeParams" and "acceptedParams", they are like that in the params interceptor. I think we can rename them. acceptedParams was added on r1817 and nobody can be using it, as the bug it had made it useless.
On 8/14/08, Dave Newton <[EMAIL PROTECTED]> wrote: > 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] > > -- "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]
