Interesting subtle bug write-up
-
Got this in a Google search for an unrelated topic, decided to read it. Pretty good read.
Bonus WTF-points for Rails changing prior code in a breaking way and swallowing what would've been an exception... (Subtle-or-not-so-subtle-bug-land ho!)
-
-
@sloosecannon said:
Rails
Thanks for warning me, I almost followed that link.
Eh, it's worth it.
Especially for laughing at the Rails guys for having a nasty change like that in a point release...
-
Especially for laughing at the Rails guys for having a nasty change like that in a point release...
Yeah.
Seems Discourse and Ruby are natural allies
-
It would almost make me feel sorry for them...
Almost.
FWIW, it looks like that breaking change was introduced for security reasons. So I suppose that's a slightly valid reason. Still, if you're gonna do that, it should be at the very top of your patch notes in very loud font:
THIS CHANGE MAY BREAK YOUR API: All AJAX requests require CSRF Tokens!
-
I don't see why they couldn't have had it spit out an (obvious, easy to find) error/warning in addition to resetting the session, to prevent exactly the situation that the author was in.
-
Well yes, that too. Instead of swallowing it, they shoud've spit out a loud warning too
-
Yeah, since in theory, non-CSRF things would be interpreted as some kind of attack, that would be a good thing to know.
Filed Under: The AJAX is coming from inside the page!
-
This difference in behavior based on the hidden interaction of two concurrent processes is called a race condition. Race conditions are why sane programmers don’t program with threads or, if they do, they use shared-nothing architecture and pass all communication between the threads through a message queue written by someone who knows what they are doing (if you have to ask, it isn’t you — seriously, multithreaded programming is hard).
... or use a language that has solid threading support where it's hard to fuck up, like C#.
-
Especially for laughing at the Rails guys for having a nasty change like that in a point release...
Requiring CSRF for AJAX requests is common-sense, and the weird thing is that it wasn't doing that since day one.
Removing the exception for a CSRF failure, though, ... it'd take a lot of convincing to tell me that's the right thing to do.
-
weird thing is that it wasn't doing that since day one.
Well, it is Rails. So... is anyone really surprised?
Removing the exception for a CSRF failure, though, ... it'd take a lot of convincing to tell me that's the right thing to do.
Yeah. It's not like handling an exception is so excruciatingly hard that it means you get any real benefit from removing one. Or not having it there at all...
-
I'm guessing someone involved with Rails was like "it's so spammy when there's an attack on my site!" and incorrectly came to the conclusion that the solution was to remove the exception when it's really to get a logging library that can "fold up" identical exception messages.
-
I think it's either that or they "justified" it because of that change to AJAX causing an increase in the volume of errors.
I kinda hope it's your reason and not that, because that reason makes me feel sad inside...
-
it would silently just clear the session and re-run the request. For most sensitive operations (e.g. those which require a signed in user), this would force a signout
Sooooo... instead of a cross-site request forgery attack, this allows a cross-site logout attack.
That's a little bit better, I guess, but not ideal. Site B shouldn't be able to log me out of Site A without my permission.
-
[url=http://gta.wikia.com/wiki/Rails]MORE RAILS, PLEASE![/url]
-
It's Rails. You aren't supposed to be doing anything complex enough that you would need exceptions.
