Rails SQL injection vulnerability: here are the facts

http://blog.phusion.nl/2013/01/03/rails-sql-injection-vulnerability-hold-your-horses-here-are-the-facts/#.UOV2h_j3Giw

by blacktulip • 14 years ago

289 107 14 years ago

blog.phusion.nl

Jgrubb• 14 years ago

I swear I saw this same bug discussed on Reddit from some security mailing list last week, unless this is another SQL injection vulnerability with Authlogic.

FooBarWidget10• 14 years ago

You talking about this? https://github.com/binarylogic/authlogic/pull/341 It's the same thing.

why-el• 14 years ago

It was also emailed to the Rails Core mailing list, thats where I read about it first.

dsl3• 14 years ago

The author wastes a lot of breath refuting claims that this is a huge Rails bug, then finally comes clean 3/4 of the way in:

   Other exploitable scenarios

   Your code is vulnerable if you call Foo.find_by_whatever(bar), where bar can be
   an arbitrary user-specified hash with symbol keys.

This is a fundamental flaw that requires a very specific set of circumstances (which the Rails community is clinging to as a get out of jail free card), similar to the Python Pickle boondoggle [1], which will result in lots of application specific vulnerabilities down the road.

1. http://nadiana.com/python-pickle-insecure

FooBarWidget10• 14 years ago

> The author wastes a lot of breath refuting claims that this is a huge Ruby bug

Get your facts right first. It's not a Ruby bug. It's a Rails bug. Ruby != Rails.

Many words are used to explain how it works. That is not a refute, nowhere did I claim the bug does not exist. But the requirement for specific circumstances is a fact.

The Python pickle example is not only totally irrelevant, it is also not a security vulnerability. Pickle does exactly what it is supposed to do. You weren't supposed to unpickle arbitrary untrusted data in the first place. If you require a safer alternative, use JSON or something, but don't expect as many features as pickle provides.

In the end, whether this is a "bug" or a "huge bug" is left as an opinion. The article provides hard facts, and a little bit of commentary. Fact is, we've written a ton of Rails apps that all use find_by_* quite extensively but none turned out to be vulnerable because the case where 'foo' is a symbol hash is rare. Whether you think this is an excuse or not, I'll let you decide.

dsl3• 14 years ago

Thanks. Sorry, that was a freudian slip from just reading a rage tweet about Ruby vs. Rails.

mileszs• 14 years ago

I don't see it as a "huge Ruby bug". Could you elaborate, please?

I would understand "huge Ruby on Rails bug", though, by convention, it is still a fairly unlikely case.

Edit: I think your edit answers my "Could you elaborate" question somewhat, as you relate a Python library issue to this issue.

tptacek16• 14 years ago

Python's Pickle is insecure by design. It causes application-specific vulnerabilities the way bad crypto libraries do, by giving developers a tool to blow their feet off with. It's hard to blame Python too much for that; languages are going to provide unsafe libraries.

This, on the other hand, is just a Rails bug. It has a simple fix. That fix is provided transparently by Rails. It isn't going to cause "lots of application specific vulnerabilities" because nobody is going to care about it 6 months from now, except as yet another reason to keep Rails at the most recent version. This is a problem no different from that faced by people on J2EE stacks.

So I disagree with both of your points.

dsl3• 14 years ago

Note that I didn't refer to the Pickle issue as a bug. I think its quite a useful feature, but it's still a pointy stick that many have fallen on.

jeswin• 14 years ago

There should be a higher bar for libraries shipping with languages, than those shipping with frameworks like Rails.

dasil0034• 14 years ago

It's not a matter of anyone egotistically "clinging" to something. The fact is that user params come in with string keys, so an app has to be doing something unusual to be vulnerable. This drastically cuts down the number of vulnerable apps. It is what it is.

amalag8• 14 years ago

The fact of the matter is people don't know enough about Rails to know what this means and like to go with the easier to spread story: "Rails vulnerability found in all versions related to user input"

Sounds much more sexy and they can do more Rails bashing that way. The fact is true as you and the article says, it's pretty obscure. In addition to being obscure, you need the secret session key.

amalag8• 14 years ago

Yes except that bar cannot be from params since params will never return symbols as keys.

So it is pretty obscure unless you take user input and do something pretty special to make it return symbols and then run find_by_whatever on it.

tptacek16• 14 years ago

This post is written very confidently. I am less confident than the author of this post that the likelihood of a generic exploit via application input (that is, not using forged cookies) is remote.

Wish there was more I could say right now. I'm not saying I have a curl command that exploits the vulnerability. I'd just be careful about making assumptions about this bug.

FooBarWidget10• 14 years ago

Not disagreeing with you there. After all there's no way to prove that something doesn't exist. What's why I wrote that everyone should upgrade, just in case. :)

The goal of the article is not to defend Rails. It is to inform about the nature of the vulnerability and to replace the feeling of panic with rational thoughts.

judofyr3• 14 years ago

I'm not quite sure what you're hinting at. I know there's been found a DoS-exploit related to the parameter handling (expect a new security release soon), but at least it seems to be safe to say that this bug can't be triggered from `params`.

tptacek16• 14 years ago

We may be talking about the same parameter handling condition, or two different ones.

PommeDeTerre5• 14 years ago

This smug attitude is something we've seen a lot from the Ruby, and especially the Rails, community, and it always burns them.

They had this attitude when it came to the maintainability of Ruby apps. They'd say that Ruby code was much more maintainable than Java code, for instance. Now that we've got some Ruby apps that are several years old, and that have been worked on by a number of different people, it has become quite apparent that Ruby code is much less maintainable, over the long term, than Java code is.

This same attitude was shown when it came to Ruby's performance. Many of us had serious misgivings about just how poorly it performed, and the suitability of Ruby on Rails for anything beyond simple sites. Then when some Ruby on Rails sites started facing moderate traffic, they basically collapsed. The answer from the Rails advocates was to "throw more hardware at it".

We even saw this same attitude when it came to this very issue. We'd heard about how Ruby on Rails is far more "secure" than alternatives. Yet that's proven not to be the case.

When Rubyists make claims, it's best to doubt what they're saying, and to question every single detail about it. They have given themselves a bad reputation for making incorrect statements.

tptacek16• 14 years ago

I wasn't making a language war claim. Every platform has security bugs.

FooBarWidget10• 14 years ago

Not sure where you got the idea that it's a "smug attitude". The article explains the facts and adds some commentary, nowhere did it claim that the bug does not exist or that one should not upgrade. Its intention is provide the facts and details of the exploit to counter the FUD coming from incomplete information and knee-jerk reactions. The bug is taken very seriously.

phillmv2• 14 years ago

I'm surprised people are still making these arguments. Well, that's okay - don't use it then! We'll figure it out on our own.

I'm interested on the "maintainability" claim, though, because that one is new to me. Are there any blog posts that make this claim with some kind of example?

no_more_death• 14 years ago

It's unfair to say that the core language is flawed. Maintainability depends on how the language features are utilized. Some code uses monkey-patching gratuitously and that can harm maintainability, but judicious use of monkey patching is not a problem. On the other hand, maintaining a several gigabyte Java project can be a nightmare. Code bloat is certainly a factor here. Java is probably better for larger, more ponderous, enterprisey projects. Ruby is designed for rapid, agile development. So we don't need a "war" here; they serve different, equally important niches.

I agree that there are performance issues with Ruby implementations such as the MRI. That's really part and parcel of an immature implementation, that is, it hasn't received the millions of developer man hours spent fine-tuning it, such as the Java ecosystem has received. However, I would observe that it's still easy to get Java performance very wrong. It turns out that efficient GC is hard -- really, really hard.

rimantas• 14 years ago

  > Yet that's proven not to be the case.

You have a proof of that? Care to share? How does one even prove framework X being more secure than framework Y?

  > When Rubyists make claims, it's best to doubt what they're
  > saying, and to question every single detail about it. They have
  > given themselves a bad reputation for making incorrect
  > statements.

Would you mind to reveal, what kind of -ist you are, and why the -ists you represent make only absolutely true claims? Or can we just agree, that this kind of generalization is silly and pointless?

dasil0034• 14 years ago

Wow, you have a serious chip on your shoulder. Nothing you're saying here is more than a gross stereotypical generalization that's no better than whatever ignorant flames you read that shaped your opinion. Are you perhaps one of the Java devs that DHH poked in the eye back in the day?

petercooper2• 14 years ago

Maybe. If so, it's only slightly more bearable than the attitude of many observers towards what or how they think "the Ruby community" collectively believe, do, or act.

There's no singular Ruby community anymore than there's a C, PHP or "Linux community" whose behavior can be collectively judged.

bstar772• 14 years ago

How on earth is the article smug? The author responded to the controversy with clear and concise examples demonstrating that the issue is relatively minor and very easy to address if you think you are susceptible. Additionally, most people seem to be using Devise over Authlogic now-a-days. Not saying it's ok to have bugs surfaced in less popular gems, but this thing isn't going to bring the internet down.

derwiki2• 14 years ago

Our Rails site successfully served 45k simultaneous requests for the National Moment of Silence two weeks ago. I call that moderate traffic, and we didn't fall over. Please stop assuming that no one can make Rails scale just because there are some popular stories of it not scaling.

rjzzleep• 14 years ago

yeah well, tell you what. 70% of what i saw since coming to the us are enterprise .net applications(yeah, i'm not in silicon valley or nyc). the other 20% were dated oracle applications.

The security practices i saw are nothing short of ridiculous. If you'd take all the rails vulnerabilities and put them in a pot, and compare them to the holes i've seen in the little time i looked at them it's like comparing a secure vault to swiss cheese.

granted, saying x is secure, because y is inherently insecure is not a valid argument, but i've seen people first hand use these exaggerated advisories to justify that their bug ridden insecure enterprise stack is more justifiable for the enterprise. these people did not know pbkdf2, bcrypt, and even the sql injection whitepaper by microsoft(which btw is a joke).

wanna write secure code? NEVER assume someone else is going to secure it to you. fact is, good advisories and quick reaction to those are not a weakness but a strength.

You want to know the state of encryption in the enterprise world? Get a recent pastebin hack dump. Databases with passwords in plaintext, passwords in md5.

as for your sites basically collapsed? yes they did, it got popular during ruby 1.8.5 ffs. what the hell do you expect? it's slow? well, that's why theres lot's of custom c around. but anyway, since then many many things changed.

Java maintainable? In which world do you live? 8 different layers of OO abstractions 20 levels of dot notation, in which world is that maintainable? Notice how people move to groovy, scala, and other things, not plain java? ever wondered why?

You want highly reliable systems, your answer is not going to be java. Try erlang, but that one is GENUINELY slow.

Oh wait, did I even mention jruby? There you go, all the Java you want, kinda.

BUGHUNTER• 14 years ago

As a developer it is best to enable yourself to collect some relevant facts about technical questions and come to a decision based on that data.

Basing an argument about technical things on "what people say" will not succeed, no matter what your goals are.

amalag8• 14 years ago

So you don't have a curl command that exploits it, but you don't believe the authors. That leaves us with what?

tptacek16• 14 years ago

Apparently, it leaves you waiting for an upcoming Rails advisory.

mikeash• 14 years ago

This leaves us with assuming that remotely triggered bugs can be exploited until proven otherwise, the way a sane person would.

catfish• 14 years ago

Meet the new boss, same as the old one...

homakov2• 14 years ago

There are a bit more vulns hanging around 'params' http://homakov.blogspot.com/2013/01/rails-security-digest-el...

benmmurphy2• 14 years ago

I can confirm that there is a generic way to exploit this. There is also a denial of service vector that is currently unpatched and depending on the way you have written your app there is a remote code execution issue as well which is unpatched (i don't think any real apps would be vulnerable to this.. but who knows). i suspect dos + rce use the same vector that tptacek has found.

mef• 14 years ago

This bug is pretty edge, but if there was an easy way for a user to put a symbol in the params hash the bug would be a pretty gnarly universal SQL injection vulnerability for all versions of Rails.

Though if that was easy it would most likely have been caught much, much earlier as there are a multitude of find helpers that allow literal SQL to be injected.

FooBarWidget10• 14 years ago

Early versions of Rails explicitly made the decision to never symbolize user inputted hashes because symbols are never garbage collected. This would allow memory to grow unbounded. As a result Rails stores all user hash keys as strings.

phillc2• 14 years ago

Thank you... I always wondered why that was true.

alxndr• 14 years ago

Ah, I also wondered about that. Is that why HashWithIndifferentAccess uses strings internally?

peterwoo• 14 years ago

So it's just serendipitous that this isn't the obvious and massive vulnerability it initially seemed to be.

scanr• 14 years ago

There are suggestions that it is possible (potentially easy) for a user to get symbols into the params hash.

phillc2• 14 years ago

If you are scanning your code base for find_by_'s, make sure to look at your find_all_by_'s as well if you have them.

awj• 14 years ago

...and while you're at it, rewrite those into some other form. Everything but the find_by_* dynamic finders is going to be deprecated soon anyways.

http://edgeguides.rubyonrails.org/4_0_release_notes.html#ext...

bhauer• 14 years ago

If I understand this correctly, given that a private key is needed to successfully modify the signed HTTP cookie, for a closed-source project using Ruby, Rails, and Authlogic, this vulnerability might be better characterized as an "unintentional backdoor."

numbsafari• 14 years ago

Yep, but even if you have a closed-source project you should avoid checking in any secret keys or other credentials to your source control system.

macca321• 14 years ago

I wish ruby had named the Hash class properly. It makes reading the article slightly confusing. Hash != Hashtable

tabbyjabby• 14 years ago

Of course the Hash class in Ruby implements a hash table. What makes you think it doesn't?

mike• 14 years ago

I believe that macca321 is wishing that, to avoid confusion, the term "hash" was reserved for referring to an individual hash code and that "hash table" was used for the data structure and the Ruby class that implements it.

__david__4• 14 years ago

You can blame Perl for that. Ruby took "Hash" from Perl's name for its native hash tables.

jrochkind13• 14 years ago

thanks for this, phusion.

I have no idea why THIS vulnerability is getting so much attention. There have actually been OTHER Raisl vulnerabilities in the past 6-8 months which were _more dangerous_, but did not really get attention.

The Rails team did NOT help by being very vague about the nature of the problem in their announcement. I imagine they were trying to not reveal the method of exploitation; but it has just led to the current hysteria instead. They would have been better off being taking the extra time to be fully transparent about the nature of the vulnerability -- developers need to know to assess their own risk, as well as to judge the quality of Rails (how stupid was the problem exactly, what does it say about Rails etc?), etc. By being vague about it, Rails core team has just led to everyone assuming the worst, and the current weird hysteria.

Yes, it's a vulnerability which COULD be dangerous, and it's hard to tell FOR SURE if your app is vulnerable (perhaps due to code in gem dependencies), the only safe thing to do is update to a patched version. But the chances that your app is vulnerable are pretty small (if you aren't using AuthLogic; if you are, you can update auth_logic and fix it whether or not you update rails). And past recent vulnerabilities which were actually MORE dangerous have not received this level of attention.

trustfundbaby• 14 years ago

> I have no idea why THIS vulnerability is getting so much attention

Its getting attention because its the third SQL vulnerability in 7 months. It just feels like Rails is mature enough at this point that it shouldn't have to be going through this now

catch23• 14 years ago

I don't see how after a project matures, there should be no exploitable bugs? Even Java still has serious bugs on a yearly basis... The only secure systems are the ones nobody uses.

jrochkind13• 14 years ago

That's potentially legit (people could argue about whether these rails vulnerabilities represent something to worry about in Rails, or whether even mature robust projects will still have bugs, including security bugs).

But the fact remains that THIS vulnerability isn't NEARLY as dangerous as some of those OTHER ones you mention in the last 7 months, but those other ones people mostly ignored, and THIS one they're going crazy thinking they need to fix right away.

What I think about the general question? Most (all?) of those Rails SQL injection bugs actually are related to a similar underlying design: Attempt to create methods with 'variable signatures', where you can give it a string OR a hash, or a list of various strings and hashes, and all of those things mean different things.

I think all of the Rails SQL injection bugs are actually related to variable arguments like that. When those variable argument methods were designed in Rails, it's probably safe to say nobody realized there were security implications, that it opens you up to a whole class of bugs where someone puts a hash where you expect a string and it change the semantics of the method call because your variable argument interpreting logic had some flaws. In retrospect, it's possibly not a great thing to do.

But Rails is not the only offender here, it's a pretty common design pattern in lots of ruby -- I think it's probably a mistaken one, but it is one that developers tend to like the convenience of.

FooBarWidget10• 14 years ago

ActiveRecord was basically rewritten in Rails 3 due to the introduction of ActiveRelation. That could have introduced security issues that weren't there before.

tptacek16• 14 years ago

It's not an "SQL injection" vulnerability; it's a logic error that happens to impact the database rather than, say, OS command execution or file handling or the object space. If you're trying to get a handle on what the actual security issues and trends are with Rails, the "SQL" in the title of this vulnerability is very misleading.

tptacek16• 14 years ago

This is a comment written under the confident assumption that the Phusion article is correct. I think the article is wrong, and wrong in a way that more or less invalidates this whole comment.

You should take this bug very seriously, and also pay close attention to Rails security releases for the next couple of weeks.

ajross4• 14 years ago

I'm starting to wonder if the DRY-uber-alles notion popularized by Rails is turning into a security antipattern. It's just too easy for code like this (intended to "make things work cleanly") to forget all the needed checks and all the constraints on the design.

The end user ends up repeating themselves less, but that means that the library code ends up getting used in lots of places and for lots of purposes that the author didn't necessarily think through...

__david__4• 14 years ago

It also means that when a bug is found in that code that you only have to fix it in one place, not 75,000 random places where it's duplicated, and probably slightly differently each time, making it hard to search for.

ajross4• 14 years ago

That seems to be belied by this very case, no? Rails had two ways of doing something, one of which forgot where the input could come from...

tptacek16• 14 years ago

The vulnerability here was caused by deliberately introduced complexity --- features for features sake. You're tempted to blame the dynamic finders (which have fallen far out of fashion anyways) but really the issue here is Rails parameter handling, which was not designed to minimize typing for Ruby programmers.

benmmurphy2• 14 years ago

the other thing to note is that if your application is deserializing untrusted input using Marshall.load and calling methods on the deserialized objects then there is a chance that you are vulnerable to arbitrary ruby code execution. in that case an attacker could execute whatever sql he wanted to anyway.

digitalboss• 14 years ago

Excellent post - clear and with example.

But - I'm confused why one of the readers on the post (Jonas) is calling this a PR Stunt?

FooBarWidget10• 14 years ago

After giving many of the comments here a thought, I've written a follow-up article "Securing the Rails session secret" in which different ways to secure the session secret are considered. Feedback is more than welcome. http://blog.phusion.nl/2013/01/04/securing-the-rails-session...

Rails SQL injection vulnerability: here are the facts

Related Stories