CODEWITHSUNDEEP
ruby-on-railsrubysql-injectionruby-on-rails-2
Richa Sinha
Richa Sinha

Reputation: 1456

What is the best possible way to avoid the sql injection?

I am using ruby 1.8.7 and rails 2.3.2

The following code is prone to sql injection

params[:id] = "1) OR 1=1--"
User.delete_all("id = #{params[:id]}")

My question is by doing the following will be the best solution to avoid sql injection or not. If not then what is the best way to do so?

User.delete_all("id = #{params[:id].to_i}")

Upvotes: 0

Views: 321

Answers (4)

Michael Chaney
Michael Chaney

Reputation: 3031

The other answers answer this well for Rails and it'll work fine if you follow their suggestions. In a more generic setting when you have to handle this yourself you can typically use a regular expression to extract a value that's in an expected format. This is really simple with an integer id. Think of it like this:

if params[:id] =~ /(\d+)/
  safe_id = $1.to_i
  # do something with safe_id now
end

That gets a little more complicated when you're handling strings and arbitrary data. If you have to handle such data then you can use the quoting methods available for the database adapters. In Rails this is ultimately rolled into a consistent interface:

safe_string = ActiveRecord::Base.connection.quote(unsafe_string)

For most database systems this will handle single quotes and backslashes in a special manner.

If you're outside of Rails you will have to use the quoting methods specific to your database adapter, but usage is quite similar.

The takeaway:

  1. If your data has a particular format, enforce the format with a regular expression
  2. Otherwise, use your database adapter's quoting function to make the data "safe" for use in a query
  3. Rails will handle most of this for you if you properly use the various methods and "conditions"

Upvotes: 1

Fer
Fer

Reputation: 3347

Use the rails methods to pass your where options. You can always hardcode them, as in the example that you give, but the usual way would be something like:

User.where(:id => params[:id]).delete_all
User.where("id = ?", params[:id]).delete_all
User.where("id = :id", :id => params[:id]).delete_all

They are well tested and in case a new vulnerability is detected, an update will fix the problem and your code will not need to be changed.

By the way, if you just want to delete 1 record based on its id, what I would do is:

User.find(params[:id]).destroy

Upvotes: 0

apneadiving
apneadiving

Reputation: 115511

What about:

User.where(id: params[:id]).delete_all

Ok sorry for Rails 2.x its:

User.delete_all(["id = ?", params[:id]])

Check doc

Btw, be sure you want to use delete_all instead of destroy_all, the former doesn't trigger callbacks.

Upvotes: 2

anusha
anusha

Reputation: 2135

You can use this also 

User.delete(params[:id])

Upvotes: 1

Related Questions