CODEWITHSUNDEEP
ruby-on-railspostgresql
UKatz
UKatz

Reputation: 613

Avoiding Potential SQL Injection

I am building an API with RoR 4 and PostgreSQL. I want to give my users some search functionality through the index action in my controllers. To this end I built the following function.

query_params is the result of params.permit in one the controllers.

The ouput of this funcion is then sent to the active record where function.

def searchable (query_params)
    query = ''    
    query_params.each do |key, value|
      value = value.to_s # Ensure this is a string or you will have crashes below
      # Equals any of multiple values
      if value[/\A(mul-)/]
        value[0..3] = ''
        query += "#{key} IN (#{value}) AND "
      # Not a single value
      elsif value[/\A(<>)/]
        value[0..1] = ''
        query += "(#{key} <> #{value}) AND "    
      # Bigger or smaller than a single value
      elsif value[/\A[><]/]
        operator = value[/\A[><]/]
        value[0] = ''
        query += "(#{key} #{operator} #{value}) AND "    
      # Equals a single value
      else
        query += "#{key} = '#{value}' AND "
    
      end
    end
    query = query[0..-5] # removes the last AND
  end

I am worried this method opens my DB up to sql injections. Am I right? If you have an example of a possible injection that would be great. Finally, is there anything I can do to block injections?

Thanks!

Upvotes: 0

Views: 454

Answers (2)

UKatz
UKatz

Reputation: 613

After reading @Miguel Cunha's answer I rewrote the code and it is now safe. I wanted to upload the changes, as I think it might be useful for others to compare. In any case I recommend looking at gems (and perhaps Elastic Search) first.

  # Permitted params makes sure keys cannot cause SQL injections!
  def searchable (query_params)
    query_keys = '' # Using array to prevent SQL injection
    query_values = {}  
    query_params.each do |key, value|
      value = value.to_s # Ensure this is a string or you will have crashes below

      # Equals any of multiple values
      if value[/\A(mul-)/]
        query_keys += "#{key} IN (:#{key}) AND "
        value[0..3] = ''
        query_values[key.to_sym] = value.split(',')
      # Not a single value
      elsif value[/\A(<>)/]
        query_keys += "#{key} <> :#{key} AND "
        value[0..1] = ''
        query_values[key.to_sym] = value
      # Bigger or smaller than a single value
      elsif value[/\A[><]/]
        operator = value[/\A[><]/]
        query_keys += "#{key} #{operator} :#{key} AND "
        value[0] = ''
        query_values[key.to_sym] = value   
      # Equals a single value
      else
        query_keys += "#{key} = :#{key} AND "
        query_values[key.to_sym] = value   
      end
    end
    query_keys = query_keys[0..-5] # removes the last AND
    query = [query_keys, query_values]
  end

Upvotes: 0

Miguel Cunha
Miguel Cunha

Reputation: 663

You should use the default's ActiveRecord query functions, since they protect you from most of programmers' mistakes. Also, you should always verify the users' input before you pass it to DB queries.

The point about RoR and its Models is to abstract you from DB, making your app (almost) DB-agnostic.

By the way, this site may be helpful: http://rails-sqli.org/

EDIT: Another thing: never pass strings directly to Model's queries. Instead, just pass a hash. Example:

Don't use:

Model.where("id = #{params[id]}")

Instead use:

Model.where(id: params[id])

Upvotes: 3

Related Questions