6 ms·

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 th

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 generally happens in one of two says: 1) (most common) You have a SQL statement that takes a user-provided parameter and you compose your SQL statement as a string, including that parameter (eg., sql = "SELECT * FROM person where id = " + form.id, or similar). This is typically solved by using parametric, prepared statements. Basically, you prepare a SQL statement that contains "?" for the parameter values and then bind values to the statement. 2) (Common in ORM frameworks) A user provided string is used to compose some other (non-parameter) piece of the SQL statement, such as a column or table name. This is usually caused by laziness. Rather than combining the string provided by the user (form values, URL components, etc.) you should instead look up the string to use from some internal data source, such as a list of domain classes, etc., and use that instead. In that way, the data that is user provided is kept entirely separate from data that will be executed. You'll hear a lot of people talk about "why isn't this being escaped". And, frankly, it's a good question. But the real question is "why are you trusting data that could come from anywhere on earth?". Don't take it for granted that only your users will be sending queries to your application. The code you write for Django (Post.objects.get(pk=request.GET['id']) is only secure from 1 and 2 if the framework is written in an appropriate manner to avoid trusting user provided data. 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. ps. This type of problem applies to any kind of "data that is executable", be it strings passed to an "eval" function or strings passed to a web browser. SQL is just a giant eval() function.

plaguuuuuu 14y ago
>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?
- numbsafari 14y ago
  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.
  - prodigal_erik 14y ago
    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?
  - rapind 14y ago
    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.
    - numbsafari 14y ago
      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.
charliesome 14y ago
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.
- numbsafari 14y ago
  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.
  - polemic 14y ago
    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.
    - numbsafari 14y ago
      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?
      - trainspottin 14y ago
        um um um you sound like an idiot. shut up already you've failed to redeem and save face.
        numbsafari 14y ago
phillmv 14y ago
>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?
- numbsafari 14y ago
  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.
  - deleted 14y ago
    [deleted]
  - dhimes 14y ago
    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...
jrochkind1 14y ago
> 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.
- numbsafari 14y ago
  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?

plaguuuuuu 14y ago

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

numbsafari 14y ago
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.
- prodigal_erik 14y ago
  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?
- rapind 14y ago
  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.
  - numbsafari 14y ago
    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.

charliesome 14y ago

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.

numbsafari 14y ago
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.
- polemic 14y ago
  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.
  - numbsafari 14y ago
    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?
    - trainspottin 14y ago
      um um um you sound like an idiot. shut up already you've failed to redeem and save face.
      - numbsafari 14y ago

phillmv 14y ago

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

numbsafari 14y ago
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.
- deleted 14y ago
  [deleted]
- dhimes 14y ago
  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...

jrochkind1 14y ago

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

numbsafari 14y ago
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?