You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@whimsical.apache.org by sebb <se...@gmail.com> on 2017/06/08 10:38:02 UTC
Re: [whimsy] branch master updated: Check for clean resources.
On 8 June 2017 at 11:28, <jo...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> johndament pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/whimsy.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new 61bf3db Check for clean resources.
> 61bf3db is described below
>
> commit 61bf3db86b73122d9e014e98f9624db05104b925
> Author: John D. Ament <jo...@apache.org>
> AuthorDate: Thu Jun 8 06:28:49 2017 -0400
>
> Check for clean resources.
> ---
> lib/whimsy/asf/podlings.rb | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/whimsy/asf/podlings.rb b/lib/whimsy/asf/podlings.rb
> index 408d3e2..1a3aae2 100644
> --- a/lib/whimsy/asf/podlings.rb
> +++ b/lib/whimsy/asf/podlings.rb
> @@ -209,6 +209,7 @@ module ASF
> end
>
> def podlingStatus
> + @resource.untaint if @resource =~ /\A\w+\Z/
> incubator_content = ASF::SVN['asf/incubator/public/trunk/content']
> resource_yml = "#{incubator_content}/podlings/#{@resource}.yml"
Don't you need to handle the case where the resource cannot be untainted?
> if File.exist?(resource_yml)
>
> --
> To stop receiving notification emails like this one, please contact
> ['"commits@whimsical.apache.org" <co...@whimsical.apache.org>'].
Re: [whimsy] branch master updated: Check for clean resources.
Posted by Sam Ruby <ru...@intertwingly.net>.
On Thu, Jun 8, 2017 at 7:05 AM, sebb <se...@gmail.com> wrote:
> On 8 June 2017 at 11:49, John D. Ament <jo...@gmail.com> wrote:
>> The whole path seems a bit weird to me. I'm not 100% sure why it even
>> detects it as a possibly tainted value since the podling's information
>> should have been read externally rather than from the request.
>
> If it cannot be insecure, then just unconditionally taint.
> For example, __FILE__ should be safe to untaint without needing to check.
Agree with Sebb's advice, but content read externally should be
untrusted and verified clean before untainting. In this case, this is
content from a podlings.xml file, which any committer could put
something there.
That being said, I'm not sure it is worth it to invest in a custom
error when the content doesn't match, the security error raised should
be sufficient.
- Sam Ruby
Re: [whimsy] branch master updated: Check for clean resources.
Posted by sebb <se...@gmail.com>.
On 8 June 2017 at 11:49, John D. Ament <jo...@gmail.com> wrote:
> The whole path seems a bit weird to me. I'm not 100% sure why it even
> detects it as a possibly tainted value since the podling's information
> should have been read externally rather than from the request.
If it cannot be insecure, then just unconditionally taint.
For example, __FILE__ should be safe to untaint without needing to check.
However if there is a possibility that the value is unsecure, only
untaint if it is OK, and stop processing otherwise.
If you don't expect the value to be insecure, but are not sure, just
throw an exception with the details:
raise ArgumentError, 'unexpected value ...'
Or return some kind of error message as the yaml so the PPMC can fix the issue.
> On Thu, Jun 8, 2017 at 6:38 AM sebb <se...@gmail.com> wrote:
>
>> On 8 June 2017 at 11:28, <jo...@apache.org> wrote:
>> > This is an automated email from the ASF dual-hosted git repository.
>> >
>> > johndament pushed a commit to branch master
>> > in repository https://gitbox.apache.org/repos/asf/whimsy.git
>> >
>> >
>> > The following commit(s) were added to refs/heads/master by this push:
>> > new 61bf3db Check for clean resources.
>> > 61bf3db is described below
>> >
>> > commit 61bf3db86b73122d9e014e98f9624db05104b925
>> > Author: John D. Ament <jo...@apache.org>
>> > AuthorDate: Thu Jun 8 06:28:49 2017 -0400
>> >
>> > Check for clean resources.
>> > ---
>> > lib/whimsy/asf/podlings.rb | 1 +
>> > 1 file changed, 1 insertion(+)
>> >
>> > diff --git a/lib/whimsy/asf/podlings.rb b/lib/whimsy/asf/podlings.rb
>> > index 408d3e2..1a3aae2 100644
>> > --- a/lib/whimsy/asf/podlings.rb
>> > +++ b/lib/whimsy/asf/podlings.rb
>> > @@ -209,6 +209,7 @@ module ASF
>> > end
>> >
>> > def podlingStatus
>> > + @resource.untaint if @resource =~ /\A\w+\Z/
>> > incubator_content = ASF::SVN['asf/incubator/public/trunk/content']
>> > resource_yml = "#{incubator_content}/podlings/#{@resource}.yml"
>>
>> Don't you need to handle the case where the resource cannot be untainted?
>>
>> > if File.exist?(resource_yml)
>> >
>> > --
>> > To stop receiving notification emails like this one, please contact
>> > ['"commits@whimsical.apache.org" <co...@whimsical.apache.org>'].
>>
Re: [whimsy] branch master updated: Check for clean resources.
Posted by "John D. Ament" <jo...@gmail.com>.
The whole path seems a bit weird to me. I'm not 100% sure why it even
detects it as a possibly tainted value since the podling's information
should have been read externally rather than from the request.
On Thu, Jun 8, 2017 at 6:38 AM sebb <se...@gmail.com> wrote:
> On 8 June 2017 at 11:28, <jo...@apache.org> wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > johndament pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/whimsy.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> > new 61bf3db Check for clean resources.
> > 61bf3db is described below
> >
> > commit 61bf3db86b73122d9e014e98f9624db05104b925
> > Author: John D. Ament <jo...@apache.org>
> > AuthorDate: Thu Jun 8 06:28:49 2017 -0400
> >
> > Check for clean resources.
> > ---
> > lib/whimsy/asf/podlings.rb | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/whimsy/asf/podlings.rb b/lib/whimsy/asf/podlings.rb
> > index 408d3e2..1a3aae2 100644
> > --- a/lib/whimsy/asf/podlings.rb
> > +++ b/lib/whimsy/asf/podlings.rb
> > @@ -209,6 +209,7 @@ module ASF
> > end
> >
> > def podlingStatus
> > + @resource.untaint if @resource =~ /\A\w+\Z/
> > incubator_content = ASF::SVN['asf/incubator/public/trunk/content']
> > resource_yml = "#{incubator_content}/podlings/#{@resource}.yml"
>
> Don't you need to handle the case where the resource cannot be untainted?
>
> > if File.exist?(resource_yml)
> >
> > --
> > To stop receiving notification emails like this one, please contact
> > ['"commits@whimsical.apache.org" <co...@whimsical.apache.org>'].
>