Rails 5 SQL Injection
Asked Answered
E

1

17

I've read about this for some time now on various SO threads, guides, etc... but all the answers are conflicting and contradictory.

It seems there's many similar methods, and a lot of the answers say to use a different one.

  • sanitize
  • sanitize_conditions
  • sanitize_sql
  • sanitize_sql_array
  • sanitize_sql_for_assignment
  • sanitize_sql_for_conditions
  • sanitize_sql_hash
  • sanitize_sql_hash_for_assignment
  • sanitize_sql_hash_for_conditions
  • sanitize_sql_like

I'm trying to write a 'raw query' adapter that lets me run raw Postgres queries, but allowing me to insert my own parameters that come from dangerous user input.

I can't use AR in these few instances because I'm doing complex lat/long calculations, aggregate functions, complex subqueries, etc.

So far I have tried 2 approaches:

Method 1

For this method, I don't know if sanitize is the best option of the above, or if it will work in 100% of cases... (I'm using Postgres only)

class RawQuery

  def exec(prepared, *params)
    prepared = query.dup
    params.flatten.each_with_index do |p, i|
      prepared.gsub!("$#{i + 1}", ActiveRecord::Base.sanitize(p))
    end
    ActiveRecord::Base.connection.exec_query(prepared)
  end

end

Trivial usage example (normally it wouldn't be this simple of course, or I would just use AR):

RawQuery.new.exec('SELECT * FROM users WHERE name = $1', params[:name])

Furthermore it seems that sanitize delegates to quote. But according to this SO post it says simply wrapping things with single quotes isn't secure... so I have no idea.

Method 2

I'm not sure if this is just as secure, but it seems to use an actual PG prepared function (which I assume is 100% secure). The only problem is rails doesn't print it out to the console, nor include the SQL execution time (which breaks my profiling tools).

class RawQuery

  def prepare(query, *params)
    name = "raw_query_#{SecureRandom.uuid.gsub('-', '')}"
    connection = ActiveRecord::Base.connection.raw_connection
    connection.prepare(name, query)
    connection.exec_prepared(name, params)
  end

end

Used the same way:

RawQuery.new.prepare('SELECT * FROM users WHERE name = $1', params[:name])


Is one method more secure over another? Are both 100% secure?

My apps always extend far outside of what Rails is capable of SQL-wise and I need a good lib I can include on all my projects which I know is completely safe.

Epanaphora answered 31/12, 2016 at 18:22 Comment(0)
N
12

Using quote is safe. I read the answers on the page you linked to, and I don't see anyone saying that quote is insecure. I see your question about using "quotes". Yes, if you just put quotes around a string, that is insecure, e.g.:

q = "SELECT * FROM users where email = '#{params[:email]}'"

But using quote (the method) is fine:

q = "SELECT * FROM users where email = #{connection.quote(params[:email])}"

You could play around in the console and try your best to break it, but I don't think you'll be able to:

2.3.3 :003 > ActiveRecord::Base.connection.quote("f''oo")                                                                              
 => "'f''''oo'"

If you succeed, I'm sure the Rails team would like to know (privately)! But as you can see, the quote method does more than stick a quote at the beginning and end.

Also, since you say you are looking for an authoritative citation, the comments in the source code itself suggest that quoting user inputs is the intended purpose of these functions:

https://github.com/rails/rails/blob/2471e6391dfe71cfbb8621bdf573729d961d3209/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb#L6-L13

# Quotes the column value to help prevent
# {SQL injection attacks}[http://en.wikipedia.org/wiki/SQL_injection].
def quote(value)

https://github.com/rails/rails/blob/0f1d0b1b5254e3678abaabbebb3362a100c10262/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb#L17-L20

# Quotes strings for use in SQL input.
def quote_string(s) #:nodoc:

(Note I am showing quote_string for the comment, but you should probably use quote, which tries to figure out the data type and do something appropriate.)

By the way, here is a similar question to yours, with an answer from me in 2014, and some alternatives too: How to execute a raw update sql with dynamic binding in rails

Nystatin answered 3/1, 2017 at 21:31 Comment(4)
I see - that makes sense. So what is the main difference between sanitize and sanitize_sql_for_conditions? One other part I forgot to include in my original question is, the docs for method sanitize_sql_for_conditions mention: sanitizes them into a valid SQL fragment for a WHERE clause. The docs for sanitize say Used to sanitize objects before they’re used in an SQL SELECT statement. Does that mean they're situation dependent, and one method can't be used anywhere in the SQL statement? (in the SELECT, WHERE, GROUP BY, etc). Or can I use sanitize regardless of location?Epanaphora
It looks to me like the sanitize_* methods are all protected, so I don't think you are intended to use them. I have always understood quote to be the main public method to use for this sort of thing. In fact the simple sanitize method just calls quote (as you said). Just looking at the code, it seems like the other sanitize_* methods are really for bridging between Railsy data structures (e.g. {name: "foo", email: "[email protected]"}) and quote. They call quote for you on each of the values. *for_conditions vs *for_assignment seems mostly about using , vs AND.Nystatin
Thanks for your help! Great answer.Epanaphora
I will say that quote is for escaping values where you could instead write a literal (like strings and numbers). If your users are also providing identifiers (like table and column names), you should use something else for those. Personally I would just write a very restrictive regex, e.g. allow only letters and underscores. You can also use things like quote_table_name.Nystatin

© 2022 - 2024 — McMap. All rights reserved.