13

Is this safe from SQL injection:

Guest.where(:event_id => params[:id])

I am sending in params[:id] without doing any type of sanitization.

and in general, are all of those activerecord method safe? (like where, joins, etc..)

And if not, what is the best practise to be safe? Also, please is there any caveats/edge cases I should be aware of?

Thanks

0xSina
  • 20,973
  • 34
  • 136
  • 253

3 Answers3

23

All of ActiveRecord's query-building methods, like where, group, order, and so on, are safe against SQL injection AS LONG AS you do not pass them raw SQL strings. This is vulnerable to SQL injection:

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

When you pass a string to a query-building method like that, the string will be inserted directly into the generated SQL query. This is useful sometimes, but it does raise the danger of an injection vulnerability. On the other hand, when you pass a hash of values, like this:

 Model.where(event_id: params[:id])

...then AR automatically quotes the values for you, protecting you against SQL injection.

Alex D
  • 29,755
  • 7
  • 80
  • 126
  • 1
    Quoting doesn't protect from SQL injection. Using parameterized queries does - as in *real* parameters passed at the DB (ODBC or other) driver level, not writing a SQL script that calls whatever statement execution is used by a DB. The v4 SQL Server adapter at least is susceptible to this bug. With parameterized queries no quoting is necessary – Panagiotis Kanavos Dec 23 '15 at 16:29
  • 1
    @PanagiotisKanavos, thanks for your comment. The point of my answer is that when you use ActiveRecord's query-building methods as in the second example, it is *supposed* to build a query which has no possibility of SQL injection. If that is not true, you should open a ticket on the ActiveRecord bug tracker. – Alex D Dec 24 '15 at 07:22
  • 1
    Query building is performed by the adapter, not ActiveRecord itself. This particular adapter actually creates a SQL string that declares and assings parameters before executing a dynamic SQL string - moving the point of injection from the query itself to the parameter assignments. This causes interesting failures when end users enter emojis like or smileys. A simple ODBC call would have avoided the problem and wouldn't even require sanitization or quoting. Which makes me wonder, how do the other adapters handle server calls? – Panagiotis Kanavos Dec 24 '15 at 08:35
  • 1
    @PanagiotisKanavos Which internal component of ActiveRecord actually builds the query is not relevant to the OP's question. End users *should* be able to use AR to get data out of a database, without any danger of SQL injection, as long as they use the style recommended in this answer. If that is not true, then the AR bug tracker is the right place to raise the issue, not here. – Alex D Dec 24 '15 at 14:12
  • Some useful reading on `.order` here: https://stackoverflow.com/questions/17859880/is-activerecords-order-method-vulnerable-to-sql-injection – stevec Jan 26 '21 at 14:54
12

Yes, your code is safely being cleansed before it is run on the database. Rails protects you from sql injection by automatically sanitizing input.

THE EXCEPTION is string interpolation: 

Guest.where("event_id = #{params[:id]}") # NEVER do this

Use one of these 2 options instead:

Guest.where(:event_id => params[:id]) # if you want pure ruby, use this
# OR
Guest.where("event_id = ?", params[:id]) # if you prefer raw SQL, use this

Check out the Rails Guide on security for more information related to sql injection as well as other common attacks.

Josh
  • 5,631
  • 1
  • 28
  • 54
  • On your 2nd option here, will rails sanitize params[:id]? – aaronmallen Nov 24 '14 at 23:47
  • The only real defense against SQL Injection is real parameterized queries. Sanitization can catch only a limited number of cases, while parameterization simply doesn't have this issue - parameters are passed as different pieces of data at the driver (eg ODBC) level, they are never part of the script. Sanitization and quoting are only workarounds – Panagiotis Kanavos Dec 23 '15 at 16:27
  • @PanagiotisKanavos it would be great to add a link on how to implement parameterized queries with ActiveRecord ! – Pak Mar 28 '16 at 14:57
  • when using string interpolation, the sql injection is high – jhgju77 May 13 '20 at 04:53
6

If you really need to use raw sql, you could use quote to prevent SQL injection

Here is an example that has been copied from here

conn = ActiveRecord::Base.connection
name = conn.quote("John O'Neil")
title = conn.quote(nil)
query = "INSERT INTO users (name,title) VALUES (#{name}, #{title})"
conn.execute(query)
Zhenya
  • 6,020
  • 6
  • 34
  • 42