5 ms·

Sigh - why oh why doesn't Rails use bind variables? Just about every DBMS I have used supports them (Sqlite, Mysql, Oracle, Sybase I know for sure) and if you c

by fendale 14y ago

Sigh - why oh why doesn't Rails use bind variables? Just about every DBMS I have used supports them (Sqlite, Mysql, Oracle, Sybase I know for sure) and if you correctly bind the variables into the query, fears about SQL Injection pretty much disappear. Then there is performance - in Oracle bind variables are essential for scalability. I read somewhere that Mysql performs better without bind variables. For most databases by caching prepared statement handles and binding / executing that handle many times, it will perform better than handling many one off queries. I have benchmarked a 20% improvement in throughput on Oracle by making just that change (which is a one liner in the JDBC driver).

judofyr 14y ago
This has nothing to do with bind variables. It was not a bug in how variables were bound/escaped. The bug was that Rails treated a variable (which sometimes is a user input) as a column name in certain cases.
- fendale 14y ago
  I admit I had not read the actual bug report before I commented, but I just did and it reads: >Impacted code directly passes request params to the `where` method of an ActiveRecord class like this: Post.where(:id => params[:id]).all >An attacker can make a request that causes `params[:id]` to return a specially crafted hash that will cause the WHERE clause of the SQL statement to query an arbitrary table with some value. For me that is a classic SQL injection attack that can be avoided 100% of the time if you use bind variables for all user input into an SQL query. Rails doesn't use bind variables - it uses something that looks similar, but it is actually rails code that escapes and concatenates the user input into the query string before executing it. This bug was in that escaping code. However, if Rails correctly bound the inputs to all queries (which is pretty easy to do) then this escaping code could be removed totally and this problem would never have appeared.
Morg 14y ago
Well maybe you should check that again. Your approach sounds like you should have stored procs instead. Using prepared statements or variable binding to fight SQL injections is not the best idea, although its widespread. In most cases where you want a prepared statement, you'd be better off using a stored proc, as you'll skip the expensive optimization every single time. MySQL is not even a real RDBMS (no ACID, no triggers, fail APIs, etc.), anyone using it should switch to PostgreSQL yesterday unless their data really doesn't matter. SQL injections are 100% avoided by user input control in the application, and the simplest way is to escape all escape characters, that may require reading a bit of doc but w/e.
- deleted 14y ago
  [deleted]
  - Morg 14y ago
    Indeed it would affect everyone just as much - still, I will spread MySQL knowledge (not hate) whether or not there is a reason for it. And I'll say it time and time again, If you think MySQL does not have major issues as a DBMS, you should not make database-related decisions as your knowledge is too limited to make a sensible decision. Just like windows. The real Windows experts can both tell you how much it's made of fail and fix your issues, the others are charlatans.
- ironchef 14y ago
  "Your approach sounds like you should have stored procs instead." Not necessarily. As one moves towards stored procs, it becomes more common to place business logic in said stored procs. This tends to go against MVC and also limits some scaling options (as your business logic then is executing in your DB). They also often will tie you to a singular DB...making moving DBs more painful. "Using prepared statements or variable binding to fight SQL injections is not the best idea, although its widespread." I read it not as _the_ best idea, but as yet another layer to potentially catch something. Belt _and_ suspenders if you will. Don't get me wrong. I'm not saying prepared statements are the greatest thing since sliced bread; however, in oracle or postgresql backed apps, they're a best practice to investigate.

by fendale 14y ago

judofyr 14y ago

This has nothing to do with bind variables. It was not a bug in how variables were bound/escaped. The bug was that Rails treated a variable (which sometimes is a user input) as a column name in certain cases.

fendale 14y ago
I admit I had not read the actual bug report before I commented, but I just did and it reads: >Impacted code directly passes request params to the `where` method of an ActiveRecord class like this: Post.where(:id => params[:id]).all >An attacker can make a request that causes `params[:id]` to return a specially crafted hash that will cause the WHERE clause of the SQL statement to query an arbitrary table with some value. For me that is a classic SQL injection attack that can be avoided 100% of the time if you use bind variables for all user input into an SQL query. Rails doesn't use bind variables - it uses something that looks similar, but it is actually rails code that escapes and concatenates the user input into the query string before executing it. This bug was in that escaping code. However, if Rails correctly bound the inputs to all queries (which is pretty easy to do) then this escaping code could be removed totally and this problem would never have appeared.

Morg 14y ago

Well maybe you should check that again. Your approach sounds like you should have stored procs instead. Using prepared statements or variable binding to fight SQL injections is not the best idea, although its widespread. In most cases where you want a prepared statement, you'd be better off using a stored proc, as you'll skip the expensive optimization every single time. MySQL is not even a real RDBMS (no ACID, no triggers, fail APIs, etc.), anyone using it should switch to PostgreSQL yesterday unless their data really doesn't matter. SQL injections are 100% avoided by user input control in the application, and the simplest way is to escape all escape characters, that may require reading a bit of doc but w/e.

deleted 14y ago
[deleted]
- Morg 14y ago
  Indeed it would affect everyone just as much - still, I will spread MySQL knowledge (not hate) whether or not there is a reason for it. And I'll say it time and time again, If you think MySQL does not have major issues as a DBMS, you should not make database-related decisions as your knowledge is too limited to make a sensible decision. Just like windows. The real Windows experts can both tell you how much it's made of fail and fix your issues, the others are charlatans.
ironchef 14y ago
"Your approach sounds like you should have stored procs instead." Not necessarily. As one moves towards stored procs, it becomes more common to place business logic in said stored procs. This tends to go against MVC and also limits some scaling options (as your business logic then is executing in your DB). They also often will tie you to a singular DB...making moving DBs more painful. "Using prepared statements or variable binding to fight SQL injections is not the best idea, although its widespread." I read it not as _the_ best idea, but as yet another layer to potentially catch something. Belt _and_ suspenders if you will. Don't get me wrong. I'm not saying prepared statements are the greatest thing since sliced bread; however, in oracle or postgresql backed apps, they're a best practice to investigate.