4 ms·

I had to think about this paper a bit. While it does describe some interesting ideas, nothing mentioned here will get past properly escaping input. That is, i

I had to think about this paper a bit. While it does describe some interesting ideas, nothing mentioned here will get past properly escaping input. That is, if you use DBI's quote method or parameterized queries in Perl or PHP's mysql_real_escape_string function or whatever is appropriate in your language and MySQL library then you'll be okay. Does that seem correct?

tptacek 17y ago
No. "Properly quoting" inputs is the worst way to mitigate SQL injection flaws. What you need to be doing is replacing your concatenated string queries with precompiled parameterized queries, where the values in your queries are replaced with placeholder strings (like "?" and ":varname"), and the values are passed in a seperate call. Every mainstream database API supports these.
- mattyb 17y ago
  For PHP, use MySQLi or PDO. http://php.net/manual/en/mysqli-stmt.prepare.php http://php.net/manual/en/mysqli-stmt.prepare.php http://php.net/manual/en/pdo.prepare.php http://php.net/manual/en/pdo.prepare.php
- ratsbane 17y ago
  I didn't say "properly quoting." I said "properly escaping" and mentioned the DBI quote method E.g. $safestring=$myDBIobject->quote($taintedstring) From the O'Reilly book "Programming the Perl DBI" "By far the most important utility method is quote(), which correctly quotes and escapes SQL statements in a way that is suitable for a given database engine." If you're going to reuse a query with different values then it makes more sense to use parameterized queries. If you're going to reuse an input string in multiple queries which don't themselves get reused then it makes more sense to use an escape function like quote. If you're using a method like DBI's "selectrow_array" which combines the "prepare," "execute," and "fetchrow_array" steps into one method call then you pretty much have to use the quote method.
  - tptacek 17y ago
    Yes. That O'Reilly book is giving extremely bad advice. You should work from the premise that there is no safe concatenated string query (regardless of the corner cases where that isn't true).
    - ratsbane 17y ago
      That is absurd.
      - tptacek 17y ago
        Here's what's happening here, Douglas. You're thinking, "I know how to quote my queries so they aren't injectable. I don't need to parameterize them." Let's stipulate that you're right. You are not the only programmer on your project --- at least not if your code means anything. Someone else is going to either (1) extend or (2) cargo-cult your query. And when they do that, one of two things is going to happen: * They're simply going to forget to quote properly. For instance, they're going to add an ORDER BY clause, and they're going to accept "ASC" and "DESC" as input from their jQuery auto-loading Ajax table widget, and you're boned. * They're going to quote using an unsafe quoting function, for instance with regular expressions, and they're going to miss truncation and character set attacks. I can tell from your "that is absurd" comment that you aren't going to be sold on any of this. That comment is also a pretty big tip-off that you're not particularly experienced --- and the only reason I say that is, I have never worked a pentest where a client quoted string queries that didn't turn out injectable. You might be the one exceptional app in the world that gets it right, in which case, thank you for proving the rule for me. People who use prepared statements and who religiously keep user input completely out of query structure (for instance, by mapping table and column names, so user input simply can't be put into query structure) don't have to think about any of these problems. Why anyone would elect to think about them is beyond me.
        ratsbane 17y ago
        So you're saying... Properly escaping queries is a bad idea because you might forget to do it and if you forget to do it then it doesn't work. Here's a funny thought, though. I remember, many years ago, doing a code review and noticing something odd. The very smart developer who always used parameterized queries had written a simple insert into table X. It was all properly parameterized. Table X, however, had an on insert trigger which called a stored proc which was about eight pages long. In that stored proc, two of the parameters were concatenated into a select which was then eval-ed, thus leading to a very serious injection possibility. So, in response to your last paragraph, yes, you still have to think about injection possibility even if you use prepared statements. Really, that should be the first rule of designing for security: ALWAYS think.

tptacek 17y ago

No. "Properly quoting" inputs is the worst way to mitigate SQL injection flaws. What you need to be doing is replacing your concatenated string queries with precompiled parameterized queries, where the values in your queries are replaced with placeholder strings (like "?" and ":varname"), and the values are passed in a seperate call. Every mainstream database API supports these.

mattyb 17y ago
For PHP, use MySQLi or PDO. http://php.net/manual/en/mysqli-stmt.prepare.php http://php.net/manual/en/mysqli-stmt.prepare.php http://php.net/manual/en/pdo.prepare.php http://php.net/manual/en/pdo.prepare.php
ratsbane 17y ago
I didn't say "properly quoting." I said "properly escaping" and mentioned the DBI quote method E.g. $safestring=$myDBIobject->quote($taintedstring) From the O'Reilly book "Programming the Perl DBI" "By far the most important utility method is quote(), which correctly quotes and escapes SQL statements in a way that is suitable for a given database engine." If you're going to reuse a query with different values then it makes more sense to use parameterized queries. If you're going to reuse an input string in multiple queries which don't themselves get reused then it makes more sense to use an escape function like quote. If you're using a method like DBI's "selectrow_array" which combines the "prepare," "execute," and "fetchrow_array" steps into one method call then you pretty much have to use the quote method.
- tptacek 17y ago
  Yes. That O'Reilly book is giving extremely bad advice. You should work from the premise that there is no safe concatenated string query (regardless of the corner cases where that isn't true).
  - ratsbane 17y ago
    That is absurd.
    - tptacek 17y ago
      Here's what's happening here, Douglas. You're thinking, "I know how to quote my queries so they aren't injectable. I don't need to parameterize them." Let's stipulate that you're right. You are not the only programmer on your project --- at least not if your code means anything. Someone else is going to either (1) extend or (2) cargo-cult your query. And when they do that, one of two things is going to happen: * They're simply going to forget to quote properly. For instance, they're going to add an ORDER BY clause, and they're going to accept "ASC" and "DESC" as input from their jQuery auto-loading Ajax table widget, and you're boned. * They're going to quote using an unsafe quoting function, for instance with regular expressions, and they're going to miss truncation and character set attacks. I can tell from your "that is absurd" comment that you aren't going to be sold on any of this. That comment is also a pretty big tip-off that you're not particularly experienced --- and the only reason I say that is, I have never worked a pentest where a client quoted string queries that didn't turn out injectable. You might be the one exceptional app in the world that gets it right, in which case, thank you for proving the rule for me. People who use prepared statements and who religiously keep user input completely out of query structure (for instance, by mapping table and column names, so user input simply can't be put into query structure) don't have to think about any of these problems. Why anyone would elect to think about them is beyond me.
      - ratsbane 17y ago
        So you're saying... Properly escaping queries is a bad idea because you might forget to do it and if you forget to do it then it doesn't work. Here's a funny thought, though. I remember, many years ago, doing a code review and noticing something odd. The very smart developer who always used parameterized queries had written a simple insert into table X. It was all properly parameterized. Table X, however, had an on insert trigger which called a stored proc which was about eight pages long. In that stored proc, two of the parameters were concatenated into a select which was then eval-ed, thus leading to a very serious injection possibility. So, in response to your last paragraph, yes, you still have to think about injection possibility even if you use prepared statements. Really, that should be the first rule of designing for security: ALWAYS think.