Return-Path: Delivered-To: apmail-struts-dev-archive@www.apache.org Received: (qmail 71325 invoked from network); 14 Aug 2008 13:00:21 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 14 Aug 2008 13:00:21 -0000 Received: (qmail 49037 invoked by uid 500); 14 Aug 2008 13:00:18 -0000 Delivered-To: apmail-struts-dev-archive@struts.apache.org Received: (qmail 48774 invoked by uid 500); 14 Aug 2008 13:00:18 -0000 Mailing-List: contact dev-help@struts.apache.org; run by ezmlm Precedence: bulk List-Unsubscribe: List-Help: List-Post: List-Id: "Struts Developers List" Reply-To: "Struts Developers List" Delivered-To: mailing list dev@struts.apache.org Received: (qmail 48762 invoked by uid 99); 14 Aug 2008 13:00:18 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 14 Aug 2008 06:00:17 -0700 X-ASF-Spam-Status: No, hits=-0.0 required=10.0 tests=SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of musachy@gmail.com designates 209.85.198.224 as permitted sender) Received: from [209.85.198.224] (HELO rv-out-0506.google.com) (209.85.198.224) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 14 Aug 2008 12:59:20 +0000 Received: by rv-out-0506.google.com with SMTP id l9so346508rvb.47 for ; Thu, 14 Aug 2008 05:59:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:to :subject:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:references; bh=mqzbA+h+D1kGOEXLd62wnJ/5M2PpaJYNBjj33j3rJT4=; b=wbkHli4gpLT256WidCpbuNGEjMxKICdE281cGNvsswPzqB0zDSdnEwroLVVxLDpkOU N/BHvahOvkz8xSGaUi7LUgT2KRvu+CuyDgQnH5OGjeodz4SRHv89WUuSxxQmGX8Q614H LiU1KHsBZHj++tVrDpOg0XO3GWiyd8OlnZ/rI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=CqDRxFL8ThL+LFDt4LL2Bb3RTYL6IPgrlW5WovDS2ECZnnVnO7zMjiwIhc9za3y8DN YPof2zNJ/9sw/xwOWOZOf1Ua1E7yzVMKhe0azvkY0f7SYLudOtGHgcZWzWgEI1mHY4P5 Tg+OHmSTeK9Bv5jJeMhhAVmYu+TXPCXP3z87I= Received: by 10.140.172.19 with SMTP id u19mr622688rve.31.1218718789299; Thu, 14 Aug 2008 05:59:49 -0700 (PDT) Received: by 10.141.87.2 with HTTP; Thu, 14 Aug 2008 05:59:49 -0700 (PDT) Message-ID: Date: Thu, 14 Aug 2008 08:59:49 -0400 From: "Musachy Barroso" To: "Struts Developers List" , newton.dave@yahoo.com Subject: Re: ParameterFilterInterceptor security issue In-Reply-To: <435441.19271.qm@web56706.mail.re3.yahoo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <435441.19271.qm@web56706.mail.re3.yahoo.com> X-Virus-Checked: Checked by ClamAV on apache.org Is it possible to get email notifications on the reviews? musachy On 8/14/08, Dave Newton wrote: > Did you fix the "Memer" spelling error I noted on the review? > > > --- On Wed, 8/13/08, Musachy Barroso wrote: > >> From: Musachy Barroso >> Subject: Re: ParameterFilterInterceptor security issue >> To: "Struts Developers List" >> Date: Wednesday, August 13, 2008, 10:41 AM >> the patch for WW-2761 was committed to xwork trunk >> >> On 8/12/08, Musachy Barroso >> 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 >> 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 >> >> >> 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 >> >>> 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 >> >>>> 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 >> >>>>>>> >> >>>>>>> > name="test" >> class="com.webapp.action.TestAction"> >> >>>>>>> > name="param-namevalue-filter"> >> >>>>>>> > name="blocked">name >> >>>>>>> >> >>>>>>> > name="params"/> >> >>>>>>> >> >>>>>>> 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 >> >>>>>>> >> >>>>>>> > name="clientCrudStack"> >> >>>>>>> > name="params.excludeParams">some >> pattern >> >>>>>>> >> >>>>>>> >> >>>>>>> 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 > > --------------------------------------------------------------------- > 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 --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org For additional commands, e-mail: dev-help@struts.apache.org