44 ms·

Forgive my ignorance, but if you're using prepared statements for all your SQL queries using user input, aren't you by definition safe from any kind of injectio

by maratd 14y ago

Forgive my ignorance, but if you're using prepared statements for all your SQL queries using user input, aren't you by definition safe from any kind of injection?

tptacek 14y ago
Of course not. This is a pernicious myth about SQL security. Many inputs to query construction can't be parameterized.
- bni 14y ago
  "Many inputs to query construction can't be parameterized." Such as? If you only use PreparedStatment (Java), or the equivalent for your language, and have a rule against, and never broken, to concatenate SQL strings together, you are safe from SQL injection.
  - j_s 14y ago
    Without dynamic sql, it is tough to do arbitrary (user-selected) columns, sorting, and server-side paging simultaneously (limiting the result set in the query instead of skipping through results in the app). SQL does not allow use of parameters in all the necessary locations in the query (result column list, order by list, etc.), and some databases are more limited than others. There are options available as one or more of the above 3 requirements is dropped: http://stackoverflow.com/questions/149380/dynamic-sorting-within-sql-stored-procedures http://stackoverflow.com/questions/149380/dynamic-sorting-wi...
    - bni 14y ago
      User selected columns is easy, just select all required columns each time and display as needed, the performance impact of this is minimal. The fact that the query need not be reparsed is probably a bigger benefit performance wise. Paging is equally easy, using ROWNUM < X in Oracle or equivalent in other RDBMS. The order by is a good point. The obvious solution here would be to always have the same sorting as default, and do the user sorting client side (by client here, I mean calling application).
      - j_s 14y ago
        Yes, the requirement of paging server-side (when a result set is ridiculously massive or the client/network is so overloaded that alternatives are impractical) is the big one: page X of Y changes depending on the sort, so sorting has to be done server-side too. Do you agree that this is a scenario requiring dynamic SQL?
        derpmeister 14y ago
        It doesn't hurt to prepare a statement, execute it once and then deallocate it. Intersect the user-selected columns with a whitelist of possible columns, use a simple flag for sorting and map it to ASC/DESC. Prepend the primary key, bind parameters, execute, harvest the results, deallocate temporary resources. This is somewhat less efficient, but if the alternative is to be open to sqli attacks it'd be my pleasure.
        tptacek 14y ago
        "Whitelist of possible columns". Congratulations, we are no longer relying on bound parameters to prevent SQL injection. THREAD FINISHED! :)
        derpmeister 14y ago
        You could do without using a CTE, it's just not as convenient most of the time. Alternatively, you could pass the column names to a function (using a prepared statement, no less) and let that function figure out which columns are valid. If your tables change frequently, this would save more time in the long run.
        tptacek 14y ago
        We are talking about manual, purpose-built functionality designed to prevent SQLI in queries that use bound parameters. Don't move the goalposts: nobody is saying it's particularly hard to avoid SQL injection, just that bound parameters aren't a panacea.
- maratd 14y ago
  > Many inputs to query construction can't be parameterized. I do not allow the client direct access to the database. They access the database through objects, which have methods that only permit parameterized input. The client cannot specify the database, table, or columns. That is hard-coded. They can only specify values. Values that are always entered through a prepared statement. In that scenario, isn't injection impossible?
  - tptacek 14y ago
    No. Of course not. In all likelihood your SQL RDBMS will only allow you to bind data. Do you paginate? What about "LIMIT" and "OFFSET"? Do you sort tables? Then you have to handle column names. Do you sort in both directions? Then you have to handle "ASC" and "DESC". And so on. Obviously, you can write simple code to do each of these safely. But then, you can write simple code to handle query parameters too. By all means, use parameterized queries. Just don't assume they're a magic totem against SQLI.
    - aw3c2 14y ago
      Could you give some actual examples of prepared statements that are injectable? I do not understand how it would work. For example the following example from http://de.php.net/manual/en/sqlite3.prepare.php http://de.php.net/manual/en/sqlite3.prepare.php : $stmt = $db->prepare('SELECT bar FROM foo WHERE id=:id'); $stmt->bindValue(':id', 1, SQLITE3_INTEGER);
      - FuzzyDunlop 14y ago
        You're missing his point somewhat. I don't know if this applies universally to SQL, but consider MySQL. $stmt = $db->prepare('SELECT bar FROM foo LIMIT :offset, :count'); ... You can't do that. You can only bind data to fields. $stmt = $db->prepare('SELECT bar FROM foo ORDER BY :column :direction'); $stmt->bindValue(':column', 'foo'); $stmt->bindValue(':direction', 'DESC'); You can't do that either, because again, it's not data. So, you end up having to do something similar to this (for the love of god, don't actually do this): $stmt->prepare("SELECT bar FROM foo WHERE id = :id ORDER BY {$column} {$direction} LIMIT {$offset}, {$count}"); Bound parameters won't save you, and the potential attack vector is there if you're not really careful. That example shows the most stupid thing you could ever do, so don't do it.
danielrm26 14y ago
Actually, no. Stored Procedures can contain dynamic SQL, which is the actual problem, so it is quite possible to have a stored procedure with SQL Injection built into it. This would yield high-performance SQL Injection and little else. The better answer is a parameterized query, which actually controls the inputs properly to ensure that no dynamic SQL is possible.
- bni 14y ago
  SP:s was never mentioned in the post you are replying to, but ofcourse if you use dynamic SQL in them (execute immediate in Oracle) it is just as bad as doing string concatenation in other languages. Just dont do that.
  - derpmeister 14y ago
    It would be a silly thing to do because you keep exactly the same problem you'd have without stored procedures. Would the client you're writing rather execute something along the lines of SELECT sp_enroll("bob\"); DROP TABLE students; --"); or SELECT sp_enroll("bob"); DROP TABLE students; --"); ? At least if you grant the user only SELECT and EXECUTE privileges and define the procedures using the SECURITY DEFINER property, you could still prevent this type of damage. (This relies on the procedures to be as strict as possible or the whole scheme essentially fails.)

by maratd 14y ago

Forgive my ignorance, but if you're using prepared statements for all your SQL queries using user input, aren't you by definition safe from any kind of injection?

tptacek 14y ago

Of course not. This is a pernicious myth about SQL security. Many inputs to query construction can't be parameterized.

bni 14y ago
"Many inputs to query construction can't be parameterized." Such as? If you only use PreparedStatment (Java), or the equivalent for your language, and have a rule against, and never broken, to concatenate SQL strings together, you are safe from SQL injection.
- j_s 14y ago
  Without dynamic sql, it is tough to do arbitrary (user-selected) columns, sorting, and server-side paging simultaneously (limiting the result set in the query instead of skipping through results in the app). SQL does not allow use of parameters in all the necessary locations in the query (result column list, order by list, etc.), and some databases are more limited than others. There are options available as one or more of the above 3 requirements is dropped: http://stackoverflow.com/questions/149380/dynamic-sorting-within-sql-stored-procedures http://stackoverflow.com/questions/149380/dynamic-sorting-wi...
  - bni 14y ago
    User selected columns is easy, just select all required columns each time and display as needed, the performance impact of this is minimal. The fact that the query need not be reparsed is probably a bigger benefit performance wise. Paging is equally easy, using ROWNUM < X in Oracle or equivalent in other RDBMS. The order by is a good point. The obvious solution here would be to always have the same sorting as default, and do the user sorting client side (by client here, I mean calling application).
    - j_s 14y ago
      Yes, the requirement of paging server-side (when a result set is ridiculously massive or the client/network is so overloaded that alternatives are impractical) is the big one: page X of Y changes depending on the sort, so sorting has to be done server-side too. Do you agree that this is a scenario requiring dynamic SQL?
      - derpmeister 14y ago
        It doesn't hurt to prepare a statement, execute it once and then deallocate it. Intersect the user-selected columns with a whitelist of possible columns, use a simple flag for sorting and map it to ASC/DESC. Prepend the primary key, bind parameters, execute, harvest the results, deallocate temporary resources. This is somewhat less efficient, but if the alternative is to be open to sqli attacks it'd be my pleasure.
        tptacek 14y ago
        "Whitelist of possible columns". Congratulations, we are no longer relying on bound parameters to prevent SQL injection. THREAD FINISHED! :)
        derpmeister 14y ago
        You could do without using a CTE, it's just not as convenient most of the time. Alternatively, you could pass the column names to a function (using a prepared statement, no less) and let that function figure out which columns are valid. If your tables change frequently, this would save more time in the long run.
        tptacek 14y ago
        We are talking about manual, purpose-built functionality designed to prevent SQLI in queries that use bound parameters. Don't move the goalposts: nobody is saying it's particularly hard to avoid SQL injection, just that bound parameters aren't a panacea.
maratd 14y ago
> Many inputs to query construction can't be parameterized. I do not allow the client direct access to the database. They access the database through objects, which have methods that only permit parameterized input. The client cannot specify the database, table, or columns. That is hard-coded. They can only specify values. Values that are always entered through a prepared statement. In that scenario, isn't injection impossible?
- tptacek 14y ago
  No. Of course not. In all likelihood your SQL RDBMS will only allow you to bind data. Do you paginate? What about "LIMIT" and "OFFSET"? Do you sort tables? Then you have to handle column names. Do you sort in both directions? Then you have to handle "ASC" and "DESC". And so on. Obviously, you can write simple code to do each of these safely. But then, you can write simple code to handle query parameters too. By all means, use parameterized queries. Just don't assume they're a magic totem against SQLI.
  - aw3c2 14y ago
    Could you give some actual examples of prepared statements that are injectable? I do not understand how it would work. For example the following example from http://de.php.net/manual/en/sqlite3.prepare.php http://de.php.net/manual/en/sqlite3.prepare.php : $stmt = $db->prepare('SELECT bar FROM foo WHERE id=:id'); $stmt->bindValue(':id', 1, SQLITE3_INTEGER);
    - FuzzyDunlop 14y ago
      You're missing his point somewhat. I don't know if this applies universally to SQL, but consider MySQL. $stmt = $db->prepare('SELECT bar FROM foo LIMIT :offset, :count'); ... You can't do that. You can only bind data to fields. $stmt = $db->prepare('SELECT bar FROM foo ORDER BY :column :direction'); $stmt->bindValue(':column', 'foo'); $stmt->bindValue(':direction', 'DESC'); You can't do that either, because again, it's not data. So, you end up having to do something similar to this (for the love of god, don't actually do this): $stmt->prepare("SELECT bar FROM foo WHERE id = :id ORDER BY {$column} {$direction} LIMIT {$offset}, {$count}"); Bound parameters won't save you, and the potential attack vector is there if you're not really careful. That example shows the most stupid thing you could ever do, so don't do it.

danielrm26 14y ago

Actually, no. Stored Procedures can contain dynamic SQL, which is the actual problem, so it is quite possible to have a stored procedure with SQL Injection built into it. This would yield high-performance SQL Injection and little else. The better answer is a parameterized query, which actually controls the inputs properly to ensure that no dynamic SQL is possible.

bni 14y ago
SP:s was never mentioned in the post you are replying to, but ofcourse if you use dynamic SQL in them (execute immediate in Oracle) it is just as bad as doing string concatenation in other languages. Just dont do that.
- derpmeister 14y ago
  It would be a silly thing to do because you keep exactly the same problem you'd have without stored procedures. Would the client you're writing rather execute something along the lines of SELECT sp_enroll("bob\"); DROP TABLE students; --"); or SELECT sp_enroll("bob"); DROP TABLE students; --"); ? At least if you grant the user only SELECT and EXECUTE privileges and define the procedures using the SECURITY DEFINER property, you could still prevent this type of damage. (This relies on the procedures to be as strict as possible or the whole scheme essentially fails.)