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.

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

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.

> > 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

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.