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:03am, Matt H. [email protected]
wrote:
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
On Sat, Apr 23, 2011 at 12:55:35AM -0700, Frederick C. wrote:
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 23 April 2011 09:02, Matt H. [email protected]
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.
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.
On Apr 23, 9:02am, Matt H. [email protected]
wrote:
def some_action
filter chain.
more stuff
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