4 ms·

SQL injection is the web-equivalent of having a stack allocated buffer overflow. We have built and use (expensive) abstractions that supposedly eliminate these

SQL injection is the web-equivalent of having a stack allocated buffer overflow. We have built and use (expensive) abstractions that supposedly eliminate these whole class of issues. With Rails, you get the expensive abstraction and apparently none of the security.

phillmv 14y ago
Yes, clearly, because Rails has introduced absolutely no other kind of default protection. Is this an argument for hand rolling your own code, or for using some other framework that is apparently immune (or, to be charitable, has a stronger security track record)?
- srdev 14y ago
  Likely its an argument to use parameterized queries which fix the SQL injection problem altogether. I'm not familiar with Ruby, but surely they support it in 2013?
  - qxcv 14y ago
    They do, but what we're talking about here is an ORM, so there will always be machine generated SQL somewhere. Or do you believe that GP is suggesting that developers use parameterised SQL queries instead of an ORM?
    - untog 14y ago
      Er, why can't the ORM use paramaterised queries?
      - jeltz 14y ago
        The problem is that ORMs like ActiveRecord really are just domain specific languages for building queries. If these DSLs use inband are carelessly constructed (e.g. they use some form of inband signaling) you can do the injection attack against the actual ORM code and make it build queries the author of the code did not intend.
        sontek 14y ago
        http://sqlalchemy.org/ http://sqlalchemy.org/ is an ORM and does not have these security issues. So it can be done.
        jeltz 14y ago
        I did not say otherwise. I said that ORMs may be vulnerable if they are carelessly constructed.
        qxcv 14y ago
        Searching for "sqlalchemy sql injection" brings up this: https://bugzilla.redhat.com/show_bug.cgi?id=783305 https://bugzilla.redhat.com/show_bug.cgi?id=783305
  - mds_ 14y ago
    Actually, no, not if you're using the mysql2 gem. There is an open issue / pull request but it doesn't seem to have much momentum. https://github.com/brianmario/mysql2/pull/289 https://github.com/brianmario/mysql2/pull/289
  - numbsafari 14y ago
    NB: what I'm about to write isn't specific to ActiveRecord, it's about SQL injection in general. So please, don't read this thinking I'm making specific claims about AR. I'm just responding to one part of the above claim. Anyhow... Parameterized queries do not "fix the SQL injection problem altogether". They solve the most common issue where someone is simply building up a full SQL statement and passing user inputs as part of the string (not as parameters). I call that "Class 1" SQL injection problem. You find this a lot in hand-rolled web apps (especially PHP apps since the legacy MySQL library hasn't been snipped out yet and most tutorials explicitly tell you to do this, even though PHP has long supported parametric SQL). However, many DB access libraries and ORMs offer facilities that generally revolve around the desire to allow the client application to customize or optimize the generated SQL created by the library (or bypass the SQL generation but leverage the library managed connection state). The API typically just trusts that you know what you are doing, blindly accepts the SQL you give it and injects it into (or replaces) whatever it generated on its own. These are the source of what I call those Class 2 injection vulnerabilities. That is, SQL gets injected in what cannot otherwise be parameterized. These can be mitigated by running a sanity check on the full SQL before it goes to the server (for example, searching it for comment strings and raising and exception or returning an error if they are detected). They can also be detected by scanning your query logs for the same things. Also, this is typically caused by a bug in your code, not the library, since it was trusting you to give it clean SQL.
tptacek 14y ago
This is silly. The stinger on this vulnerability is SQL injection, but the vulnerability itself is closer in spirit to code injection. The issue here isn't database hygiene. Parameterized queries would not have helped. There's a real critique of Rails to be leveled here (there is some fucked up stuff going on with Rails request processing), but yours isn't it.
wglb 14y ago
I am not sure I think of abstractions as featuring improved security. I recently did an (internal to client) blog post about the job of a penetration tester as "puncturer of abstractions". I do have permission to publish it publicly, so I plan to do so. But briefly, if you think of a grammar such as HTML in terms of an abstraction, the very fact that you need to encode output when creating HTML indicates that the abstraction can be "punctured". And many abstractions, such as stored procedures, don't help you out at all. There is a oft-repeated untruth that somehow stored procedures magically prevent SQLi.

phillmv 14y ago

Yes, clearly, because Rails has introduced absolutely no other kind of default protection. Is this an argument for hand rolling your own code, or for using some other framework that is apparently immune (or, to be charitable, has a stronger security track record)?

srdev 14y ago
Likely its an argument to use parameterized queries which fix the SQL injection problem altogether. I'm not familiar with Ruby, but surely they support it in 2013?
- qxcv 14y ago
  They do, but what we're talking about here is an ORM, so there will always be machine generated SQL somewhere. Or do you believe that GP is suggesting that developers use parameterised SQL queries instead of an ORM?
  - untog 14y ago
    Er, why can't the ORM use paramaterised queries?
    - jeltz 14y ago
      The problem is that ORMs like ActiveRecord really are just domain specific languages for building queries. If these DSLs use inband are carelessly constructed (e.g. they use some form of inband signaling) you can do the injection attack against the actual ORM code and make it build queries the author of the code did not intend.
      - sontek 14y ago
        http://sqlalchemy.org/ http://sqlalchemy.org/ is an ORM and does not have these security issues. So it can be done.
        jeltz 14y ago
        I did not say otherwise. I said that ORMs may be vulnerable if they are carelessly constructed.
        qxcv 14y ago
        Searching for "sqlalchemy sql injection" brings up this: https://bugzilla.redhat.com/show_bug.cgi?id=783305 https://bugzilla.redhat.com/show_bug.cgi?id=783305
- mds_ 14y ago
  Actually, no, not if you're using the mysql2 gem. There is an open issue / pull request but it doesn't seem to have much momentum. https://github.com/brianmario/mysql2/pull/289 https://github.com/brianmario/mysql2/pull/289
- numbsafari 14y ago
  NB: what I'm about to write isn't specific to ActiveRecord, it's about SQL injection in general. So please, don't read this thinking I'm making specific claims about AR. I'm just responding to one part of the above claim. Anyhow... Parameterized queries do not "fix the SQL injection problem altogether". They solve the most common issue where someone is simply building up a full SQL statement and passing user inputs as part of the string (not as parameters). I call that "Class 1" SQL injection problem. You find this a lot in hand-rolled web apps (especially PHP apps since the legacy MySQL library hasn't been snipped out yet and most tutorials explicitly tell you to do this, even though PHP has long supported parametric SQL). However, many DB access libraries and ORMs offer facilities that generally revolve around the desire to allow the client application to customize or optimize the generated SQL created by the library (or bypass the SQL generation but leverage the library managed connection state). The API typically just trusts that you know what you are doing, blindly accepts the SQL you give it and injects it into (or replaces) whatever it generated on its own. These are the source of what I call those Class 2 injection vulnerabilities. That is, SQL gets injected in what cannot otherwise be parameterized. These can be mitigated by running a sanity check on the full SQL before it goes to the server (for example, searching it for comment strings and raising and exception or returning an error if they are detected). They can also be detected by scanning your query logs for the same things. Also, this is typically caused by a bug in your code, not the library, since it was trusting you to give it clean SQL.

tptacek 14y ago

This is silly. The stinger on this vulnerability is SQL injection, but the vulnerability itself is closer in spirit to code injection. The issue here isn't database hygiene. Parameterized queries would not have helped. There's a real critique of Rails to be leveled here (there is some fucked up stuff going on with Rails request processing), but yours isn't it.

wglb 14y ago

I am not sure I think of abstractions as featuring improved security. I recently did an (internal to client) blog post about the job of a penetration tester as "puncturer of abstractions". I do have permission to publish it publicly, so I plan to do so. But briefly, if you think of a grammar such as HTML in terms of an abstraction, the very fact that you need to encode output when creating HTML indicates that the abstraction can be "punctured". And many abstractions, such as stored procedures, don't help you out at all. There is a oft-repeated untruth that somehow stored procedures magically prevent SQLi.