Hacker News Reader

This is not the problem. Please stop spreading misinformation.

ActiveRecord does escape user input.

The exploit here is that under certain obscure circumstances it is possible trick ActiveRecord into thinking the user input is an options hash passed by the caller.

From my understanding this is non-trivial to exploit on most applications, and requires passing in a Hash with symbol keys.

This is still a vulnerability that needs to be (and has been) fixed, but it is nowhere near as stupidly obvious as you are claiming.

Umm.. I didn't say that ActiveRecord doesn't escape user input. So, stop spreading misinformation about my post ;-)

The fact of the matter is, whether its in some dark edge-case or not, user-provided data is being used to compose a SQL statement that is being passed to the server. Escaped or otherwise, that's a recipe for an injection attack.

This misses the point. The problem is not that unsafe SQL is produced it is that the method signature is not always what is expected by the user.

This isn't actually an SQL injection flaw.

Um... have you read the linked posting?

>Carefully crafted requests can use the scope to inject >arbitrary SQL.

It's also titled "SQL Injection Vulnerability". Are we all missing something?

um um um you sound like an idiot. shut up already you've failed to redeem and save face.

You're the one calling people names. The guy who wrote the fix that was actually accepted by the Rails core team called this a "SQL Injection" and it has been filed in that category by numerous independent bug trackers.

I don't quite understand the angst about this defect being called a SQL injection vulnerability. The vector for the attack doesn't change the end result.

The cause might be that the API was broken, but it doesn't change the fact that a guy wrote SQL code that was injected into the middle of the rest of the SQL generated by the ORM.

> user-provided data is being used to compose a SQL statement that is being passed to the server. Escaped or otherwise, that's a recipe for an injection attack

How do you implement authentication if you can't check an email (user provided data) matches a password (user provided data, probably hashed but still)? How do you look up blog posts by a user-provided tag, without using that tag in query composition? How do you save any user provided information at all without somehow including that information in an SQL query?

You have to use escaped user provided data all the time in a real application. Any actual web developer would know that.

Simple: use parametric prepared statements instead of composing a SQL string with the user-provided data (escaped or otherwise).

This has been available in numerous database APIs for like, ever.

For example [1], [2], [3]. Any actual web developer will have read something along the lines of [4].

A lot people seem confused by my original post, which was in response to a Django user's question about how this sort of thing happens. I provided a general response which seems to have offended some people.

Yes, Rails does appear to use parameterized statements. However, when it is building those parameterized statements it's still using user provided data to build the SQL. If that weren't the case, then this wouldn't be a bug at all, would it? Of course not, so obviously it is using user-provided data in some way, otherwise an HTTP cookie's value wouldn't be getting passed to the database, would it? The prepared statement string shouldn't be composed with anything user provided.

[1] http://php.net/manual/en/mysqli-stmt.bind-param.php

[2] http://bit.ly/WlPEFR

[3] http://rubydoc.info/gems/mysql/2.9.0/Mysql/Stmt

[4] https://www.owasp.org/index.php/Top_10_2010-A1

The attack is targeting a secondary method signature that #find_by_* can hold with the express purpose of executing arbitrary SQL. That is, when #find_by_* is invoked with a hash with a key such as :select or :conditions it expects a SQL string, probably hard coded.

The bug however is that it's possible for user input, with a session hijacking, to provide that hash with symbolic key. There is no SQL injection, this is straight up arbitrary execution of SQL.

Out of curiosity, all other debates aside, would it not be helpful to have (either built-in or as a separate plugin) a way for Rails to run a simple set of sanity checks on the SQL it passes to the DB server? For example, checking to make sure that the generated SQL doesn't contain "--" wouldn't fix the underlying problem, but it could be used to prevent the exploit from ultimately working (and, if someone tried this, would alert you to that fact, assuming the error was logged).

I get that this would create some performance overhead, so it would ideally be configurable.

Yes, it's true. You'd have to pass symbol keys. It's not easy to reproduce.

>The fact that this keeps happening on Rails is the #1 reason I haven't bothered to take the time to do anything real with it. I don't have the time to read the code for the framework and I don't trust that it's written with security in mind.

What frameworks do you use? Have you performed your own audit?

Totally fair question.

I personally prefer not to use ORMs for this specific reason: they are typically way too complicated to be able to plow through the code in any reasonable way. It's also generally not that hard to design your application in such a way that using a minimalist "ORM-ish" layer of your own making isn't exactly a waste of time. I've also found that they rarely follow these best practices (it's maddening).

I have, however, had to make use of Hibernate, SQL Alchemy and Django's ORM on projects where I didn't make the calls. I'm pretty sure Hibernate uses parametric, prepared statements. I believe SQLAlchemy and Django ORM do not, but use their own escaping mechanism internally. In addition, I don't know about Hibernate and SQLA, but I'm pretty sure that Django's ORM API does make it possible to cause the framework to generate SQL using user-provided data for column/table names in a manner similar to ActiveRecord.

By way of contradicting myself, I do believe that ORMs are great for writing internal use-utilities that are one-offs or quick-and-dirty tools. In general, those cases preclude the use of autonomously provided user data for query building. For world-facing code, ORMs are risky unless you've got someone on the team who knows it and has the ability to ensure it doesn't suffer from these types of design flaws.

I'm disturbed by the fact that you are getting downvoted (here and in another comment thread on this page), largely because I largely agree. I am not an expert by any means so I try to stay atop of "best practices," but what you are saying resonates with what I thought I understood. So if you are way wrong I'd love it if somebody would come out and definitively say so.

I am using my first framework (yiiframework) at the suggestion of another dev. I still do things like

  query(user_input){
     switch(user_input){
       case(x):
         do query_x;
         break;
       case(y):
         do query_y;
         break;
   ...
   }
   }

The other dev thinks I'm nuts, but I avoid a lot of worry with this. I may lose some performance I suppose...

Totally reasonable post. Sad to see it greyed out.

After years of working in ORMs I've come around to your thinking for bigger projects. ORMs are great, but they are large, complex, and sometimes opaque project dependencies and therefor should be employed sparingly. Parameterized SQL isn't that tough to write (and Python makes it easy) and often faster. The biggest drawback: it requires a dev team comfortable with SQL or the NoSQL library bindings you're using.

I think this kind of view point is mostly influenced by having to deal with poorly designed ORMs and (tho occasionally very necessary) convoluted database designs.

> For world-facing code, ORMs are risky unless you've got someone on the team who knows it and has the ability to ensure it doesn't suffer from these types of design flaws.

This I think is wrong, for the same reason you don't want to be putting together your crypto package. If anything, these kind of security vulnerabilities demonstrate just how hard it is to get all of the subtleties pinned down. Rails is used widely and has been inspected by far more domain experts than you'd ever have on your team and yet.

Sometimes ActiveRecord gets a little in the way - especially back when has_and_belongs_to_many associations were considered best practice - but for the most part I haven't been able to empathize with these kinds of claims. AR is really flexible and gives you a lot of functionality for free.

I think you're making a valid criticism of my position. That said, I don't know that I would (personally) put SQL calls in the same boat as crypto. Crypto involves a lot of complex math and code that, if off by a single bit can wreck the whole house of cards.

In general, I think the problem that ORMs face is that they try and match every single problem thrown at them. People criticize your ORM saying "it doesn't handle egde case XYZ in my legacy data model" or "it suffers from this performance problem when somebody puts a tire boot on the server". Rather than saying "don't use an ORM to solve your unpaid parking ticket problem", the ORM team will devise a way of providing multiple method signatures in a language that loosely supports the feature so that unpaid parking tickets will always be paid prior to the server getting a boot.

Eventually the support for all these edge cases adds up to a very complex piece of software that, to your point, rivals the complexity and fragility of crypto code.

To me... it's more about saying "I have a limited set of use cases here, I don't need a leatherman to cut this noose around my neck I just need a steak knife". ActiveRecord is an impressive freaking tool and I don't begrudge anyone for using. If you ship working code using it then it did it's job.

My personal taste is to stick with simpler tools that don't have so many edge cases so I can sleep easier at night.

Suffice it to say, where you draw the line on "too complex for my taste" and where I would draw that line is probably different and the result of both our personal experiences as well as the problems we are trying to solve.

> I'm pretty sure Hibernate uses parametric, prepared statements. I believe SQLAlchemy and Django ORM do not, but use their own escaping mechanism internally.

You have no credibility to talk about database if you can't tell what kind of statements are being executed.

Um... it's been about a year since I sat around looking at the output of any of those three libraries and it wasn't worth the effort to go back and fire something up to be "certain". So I used the appropriate words "I believe" and "I'm pretty sure" to indicate that I was make statements based upon the recollections of my frail human memory ... also, those libraries are actively developed (last I checked) and could have changed since I last used them.

You have no credibility to talk about my credibility if you read too much into every single sentence I write without context.

> You are going to have problems with this whenever you are composing SQL statement with any type of user-provided data as part of the raw SQL string passed to the server.

True, but Rails is not doing that, was never doing that, and the patch has nothing to do with this. So you're talking about something unrelated to this security flaw.

I'm confused. If it's not doing that, and was never doing that, then how does an HTTP cookie's value end up injected into a SQL statement generated by Rails?

>You are going to have problems with this whenever you are composing SQL statement with any type of user-provided data as part of the raw SQL string passed to the server.

This is still, mathematically-speaking, a bug. The function is supposed to find a post by ID. If its implementation causes side effects or returns unexpected results for a certain subset of possible input data, then it doesn't conform to spec.

This becomes a question of trust. Do you trust ActiveRecord/ORM of choice to be bug-free, or do you treat it as untrusted code and basically have to worry about the implementation of data persistence in your non-DB code even though that's what ORMs are supposed to abstract away? Why is that shit running around in your codebase anyway and not part of the ORM?

I didn't say it wasn't a bug. It's a bug that indicates that ActiveRecord was written/designed in such a way that it trusts user-provided data to be executed.

And, yes, I would fully expect to be able to trust my data-abstraction layer to be bug free. Since Rails seems to have this problem regularly, I can't trust it and therefore choose not to use it for those purposes.

So, I think we agree here.

--- Edit ---

To whit, if you look at the bug report it says that the problem is when an application is passing user-provided data into the framework. They say "don't do that" and then apparently provide a patch to somehow get around if you don't (I don't know enough about the internals of rails to understand the patch).

To my mind, the problem is that Rails should be treating any data passed to it as user-provided data, rather than trusting somebody who just took a 21-day "hacker college" class to do anything other than just pass along user-provided data. The framework should be implementing this kind of security in a consistent and reliable way, rather than trusting you. That's kinda what a framework is for.

Not really a rubyist, but if you must do something as crazy as composing a query at runtime, isn't Object#tainted? the way to find out whether a string came from application code or from a user?

While I sort of agree with your argument, I question whether or not it's our responsibility to protect the world from the 21-day "hacker college" crowd. I would welcome throwing an error instead however.

There needs to be some sort of expertise cutoff and I think it's reasonable to expect in a web framework that it's user's are informed enough to avoid these sort of mistakes.

Eventually the scissors become so safe that you can't cut anything with them.

I don't think we disagree. Developers should know about these issues and no framework will ever truly protect you from these kinds of mistakes. The question is, I think, is ActiveRecord an ORM that properly mitigates these issues.

Anecdotally, it seems to have recurring problems with SQL injection.

> To whit, if you look at the bug report it says that the problem is when an application is passing user-provided data into the framework. They say "don't do that" and then apparently provide a patch to somehow get around if you don't

They never tell you not to pass in user provided data. I have no idea where you got that conclusion, but you're obviously off and running with it. Quit spreading misinformation.

The post includes a simple workaround for people who are not able to upgrade to a version that includes this security release. It's not a "don't do that" statement.

> To my mind, the problem is that Rails should be treating any data passed to it as user-provided data

It does. This is a bug. Please quit spreading misinformation.

This is the exact quote from the posting linked for this article:

---- Impacted code passes user provided data to a dynamic finder like this:

Post.find_by_id(params[:id]) ----

It later tells you to apply the "to_s" function to the "user provided data" in order to avoid the problem.

I'm unclear on how that is "misinformation".

It tells you to do that in the "Workarounds" section when talking about how the vulnerability can be mitigated. At no point do they tell you not to pass user provided data to this method.

The problem is an argument parsing bug that leads to user provided data being used as programmer provided data. Rails does not force SQL sanity off on the developer.

Whoa whoa whoa, let's be fair. As a graduate of one of these "hacker colleges" that used RoR [1], and who was a hobbyist programmer long before it, these magically generated function names (and plurals, etc) scared the hell out of me.

In all my experience with programming, I had become accustomed to the mentality of "the compiler/interpreter expects identifiers to be exactly right; it doesn't figure out what you 'really' mean". So it was frustrating to see these auto-generated methods, as I couldn't see the rhyme or reason behind where these methods were coming from.

Fortunately, I found work at a Django shop, where the framework is so much easier to follow and more explicit about how it does things.

[1] devbootcamp.com, first cohort, Spring 2012, though it was actually more like 60 days with 40 days of instruction. I'm now employed as a developer and trusted with production code.