How do I protect this Rails find_by_SQL statement from SQL injection?

Question

Thanks for your help with this one! I'm working on a legacy Rails application and I've checked a few sources but haven't been able to make them work for my situation. In Rails 2.3, I've got a search form (index.html.erb) and a controller (search.rb). The controller uses the search fields to build a string, which is saved in the variable @search_string.

    @search_string = ""

To show the results of the search, the controller uses:

    @location_matches = Location.paginate_by_sql("select * from locations where #{@search_string} order by nickname asc",  :page => params[:page], :per_page => 20)

What is the simplest way for me to sanitize the above? I tried

    @location_matches = Location.paginate_by_sql('SELECT * FROM locations WHERE #{@search_string} = ?', @search_string)

This throws the error ArgumentError (parameter hash expected). FYI, the @search_string is built with form fields like this:

FORM

    <label for="city">City</label>
    <input id="city" name="city" size="30" type="text" value="" />

CONTROLLER

    if params[:city] != ""
    @search_string << "and city like '%#{params[:city]}%' "
    if @first_term == 'y'
    @search_string = @search_string.gsub('and ', " ")
    @first_term = 'n'
    end
    end

I'm pretty sure the individual fields aren't properly parameterized, but rather than go through every field and try and fix it, I was looking for a faster solution by just tweaking the paginate_by_sql statement (I'm new to Rails and am just trying to make a quick fix since the whole app will eventually need upgrading.)

EDIT

I followed the steps in Rails, how to sanitize SQL in find_by_sql and added an initializer. I also changed my statement to the below. However, when I run my search, I get no results. I also had to delete the ascending condition because it threw an error. Any ideas are very welcome!

Initializer

    class ActiveRecord::Base  
    def self.escape_sql(clause, *rest)
    self.send(:sanitize_sql_array, rest.empty? ? clause : ([clause] + rest))
    end
    end

Controller

    query = Location.escape_sql(["SELECT * from locations WHERE #{@search_string} = ?", params[:@search_string]]) 
    @location_matches = Location.paginate_by_sql(query, :page => params[:page], :per_page => 20)

Log (Looks the same as the code that returns results)

    Parameters: {"city"=>"New York", "commit"=>"Search", "search"=>{"size_category"=>"", "state"=>""}}

Answer 1

The short answer is: you really can't do this quickly.

I'd recommend updating the search-string builder to, instead of creating just a single string, to build the array-style set of finders eg: ["field = ?", param]

eg:

search_string = ""
search_vals = []
if params[:city].present?
   search_string << "and city like ? "
   search_vals << "%#{params[:city]}%"
end
if params[:whatever].present?
   search_string << "and whatever like ? "
   search_vals << "%#{params[:whatever]}%"
end

Then when you want to do the final search, you could use it like this:

search_string = "SELECT * from locations WHERE #{search_string}"
# turns it into the array-with question-mark syntax
search_query = [search_string] + search_vals

# if paginate_by_sql takes array, you can use it straight up here:
@location_matches = Location.paginate_by_sql(search_query,  :page => params[:page], :per_page => 20)

# otherwise use your "escape_sql" variant first
@location_matches = Location.paginate_by_sql(Location.escape_sql(search_query),  :page => params[:page], :per_page => 20)