Rails - Apr 2011 - security vulnerability..controllers must 'return'

I got an email the other day from someone who has apparently built a
vulnerability
analysis tool for rails apps.

He claims (and I have no idea whether this is true, I present this purely as a
question) that a very old project of mine still hosted on github, allows
malicious
execution because controllers do not "return" at the end of each
action.

According to him, a redirect_to does not halt processing and can somehow lead to
people "executing" code.

Has anyone else heard of this or received a smiliar message? I''m mainly
just curious
because if it''s true, it would mean revising how I personally write
apps, and also how
99% of tutorials/guides are written I would think.

On Apr 23, 7:03 am, Matt Harrison
<iwasinnamuk...-ja4MoDZtUtVl57MIdRCFDg@public.gmane.org>
wrote:
> I got an email the other day from someone who has apparently built a
vulnerability
> analysis tool for rails apps.
>
> He claims (and I have no idea whether this is true, I present this purely
as a
> question) that a very old project of mine still hosted on github, allows
malicious
> execution because controllers do not "return" at the end of each
action.
>
> According to him, a redirect_to does not halt processing and can somehow
lead to
> people "executing" code.
>
> Has anyone else heard of this or received a smiliar message? I''m
mainly just curious
> because if it''s true, it would mean revising how I personally
write apps, and also how
> 99% of tutorials/guides are written I would think.
>
Well it''s certainly true that redirect_to isn''t a magic method
in that
it doesn''t affect flow of what happens afterwards ie

def some_action
  if bad_guy
    redirect_to ''/''
  end
  do_something
end

would still end up running do something. I don''t think this is a new
discovery though, I consider this a well know fact.
Also a lot of the time people do this sort of checking in a
before_filter, and rendering or redirecting from there will halt the
filter chain.

Fred
>  application_pgp-signature_part
> < 1KViewDownload
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en.

On Sat, Apr 23, 2011 at 12:55:35AM -0700, Frederick Cheung
wrote:
> 
> 
> On Apr 23, 7:03?am, Matt Harrison
<iwasinnamuk...-ja4MoDZtUtVl57MIdRCFDg@public.gmane.org>
> wrote:
> > I got an email the other day from someone who has apparently built a
vulnerability
> > analysis tool for rails apps.
> >
> > He claims (and I have no idea whether this is true, I present this
purely as a
> > question) that a very old project of mine still hosted on github,
allows malicious
> > execution because controllers do not "return" at the end of
each action.
> >
> > According to him, a redirect_to does not halt processing and can
somehow lead to
> > people "executing" code.
> >
> > Has anyone else heard of this or received a smiliar message?
I''m mainly just curious
> > because if it''s true, it would mean revising how I personally
write apps, and also how
> > 99% of tutorials/guides are written I would think.
> >
> 
> Well it''s certainly true that redirect_to isn''t a magic
method in that
> it doesn''t affect flow of what happens afterwards ie
> 
> def some_action
>   if bad_guy
>     redirect_to ''/''
>   end
>   do_something
> end
> 
> would still end up running do something. I don''t think this is a
new
> discovery though, I consider this a well know fact.
> Also a lot of the time people do this sort of checking in a
> before_filter, and rendering or redirecting from there will halt the
> filter chain.
Yes you''re correct. However from the way he put it, and the specific
line numbers he
referenced in the email to files in my old project, even something like this is
dangerous:

def some_action
  if ...
    *do stuff*
    redirect_to ''...''
  else
    *more stuff*
    redirect_to ''...''
  end

  *somehow something here will be executed even though it doesn''t exist
and should
  never be reached unless the code is modified*
end

I completely agree that if you don''t cover all the possible return
paths, you might
get undesired results. I wish I could find the email in question but it looks
like I
deleted it, because it really sounded a bit unbelievable. 

I just wanted to check that there hadn''t been some big development
recently and that I
needed to change all my habits.

On Apr 23, 9:02 am, Matt Harrison
<iwasinnamuk...-ja4MoDZtUtVl57MIdRCFDg@public.gmane.org>
wrote:
> On Sat, Apr 23, 2011 at 12:55:35AM -0700, Frederick Cheung wrote:
>
> > On Apr 23, 7:03?am, Matt Harrison
<iwasinnamuk...-ja4MoDZtUtVl57MIdRCFDg@public.gmane.org>
> > wrote:
> > > I got an email the other day from someone who has apparently
built a vulnerability
> > > analysis tool for rails apps.
>
> > > He claims (and I have no idea whether this is true, I present
this purely as a
> > > question) that a very old project of mine still hosted on github,
allows malicious
> > > execution because controllers do not "return" at the
end of each action.
>
> > > According to him, a redirect_to does not halt processing and can
somehow lead to
> > > people "executing" code.
>
> > > Has anyone else heard of this or received a smiliar message?
I''m mainly just curious
> > > because if it''s true, it would mean revising how I
personally write apps, and also how
> > > 99% of tutorials/guides are written I would think.
>
> > Well it''s certainly true that redirect_to isn''t a
magic method in that
> > it doesn''t affect flow of what happens afterwards ie
>
> > def some_action
> >   if bad_guy
> >     redirect_to ''/''
> >   end
> >   do_something
> > end
>
> > would still end up running do something. I don''t think this
is a new
> > discovery though, I consider this a well know fact.
> > Also a lot of the time people do this sort of checking in a
> > before_filter, and rendering or redirecting from there will halt the
> > filter chain.
>
> Yes you''re correct. However from the way he put it, and the
specific line numbers he
> referenced in the email to files in my old project, even something like
this is
> dangerous:
>
> def some_action
>   if ...
>     *do stuff*
>     redirect_to ''...''
>   else
>     *more stuff*
>     redirect_to ''...''
>   end
>
>   *somehow something here will be executed even though it doesn''t
exist and should
>   never be reached unless the code is modified*
> end
>
> I completely agree that if you don''t cover all the possible return
paths, you might
> get undesired results. I wish I could find the email in question but it
looks like I
> deleted it, because it really sounded a bit unbelievable.
>
> I just wanted to check that there hadn''t been some big development
recently and that I
> needed to change all my habits.
>
Sounds like rubbish to me. Pretty much the only thing you could say is
that maybe it makes it more likely that someone will add code after
that if-else-end block without thinking but even that sounds pretty
flimsy to me

Fred
>  application_pgp-signature_part
> < 1KViewDownload
-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en.

On 23 April 2011 09:02, Matt Harrison
<iwasinnamuknow-ja4MoDZtUtVl57MIdRCFDg@public.gmane.org>
wrote:
> Yes you''re correct. However from the way he put it, and the
specific line numbers he
> referenced in the email to files in my old project, even something like
this is
> dangerous:
How are you defining "dangerous"? As far as I can see, the worst is
that some of the application''s code will be run through, which
isn''t
as bad as a user being able to execute their own code.
> def some_action
>  if ...
>    *do stuff*
>    redirect_to ''...''
>  else
>    *more stuff*
>    redirect_to ''...''
>  end
>
>  *somehow something here will be executed even though it doesn''t
exist and should
>  never be reached unless the code is modified*
> end
So if there''s a situation that this could be a *problem* use an
explicit return:
>    redirect_to ''...'' and return
Not exactly a huge problem IMO.

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en.