6

I've been puzzled for a while over the difference between using a question mark, e.g.

Foo.find(:all, :conditions => ['bar IN (?)', @dangerous])

and using sprintf style field types, e.g.

Bar.find(:all, :conditions => ['qux IN (%s)', @dangerous])

in sanitizing inputs. Is there any security advantage whatsoever, if you know you're looking for a number - like an ID - and not a string, in using %d over ?, or are you just asking for a Big Nasty Error when a string comes along instead?

Does this change at all with the newer .where syntax in Rails 3 and 4?

Alexander Clark
  • 763
  • 1
  • 7
  • 15
  • i did not even know that this is possible in rails2 – phoet Oct 17 '13 at 21:53
  • 1
    @muistooshort, it seems it works at least in Rails 3. Good point about future proofing since it doesn't seem to be documented anywhere. – Alexander Clark Oct 17 '13 at 22:29
  • In 3+ I'd say `where(:bar => @dangerous)` anyway, that will do The Right Thing if `@dangerous` is a non-empty array (but alas, [something stupid](http://stackoverflow.com/a/12946338/479863) if it is an empty array). – mu is too short Oct 17 '13 at 23:58

2 Answers2

2

%s is intended for strings. The main difference is that %s doesn't add quotes. From ActiveRecord::QueryMethods.where:

Lastly, you can use sprintf-style % escapes in the template. This works slightly differently than the previous methods; you are responsible for ensuring that the values in the template are properly quoted. The values are passed to the connector for quoting, but the caller is responsible for ensuring they are enclosed in quotes in the resulting SQL. After quoting, the values are inserted using the same escapes as the Ruby core method Kernel::sprintf.

Examples:

User.where(["name = ? and email = ?", "Joe", "joe@example.com"])
# SELECT * FROM users WHERE name = 'Joe' AND email = 'joe@example.com';

User.where(["name = '%s' and email = '%s'", "Joe", "joe@example.com"])
# SELECT * FROM users WHERE name = 'Joe' AND email = 'joe@example.com';

Update:

You are passing an array. %s seems to calls .to_s on the argument so this might not works as expected:

User.where("name IN (%s)", ["foo", "bar"])
# SELECT * FROM users WHERE (name IN ([\"foo\", \"bar\"]))

User.where("name IN (?)", ["foo", "bar"])
# SELECT * FROM users WHERE (name IN ('foo','bar'))

For simple queries you can use the hash notation:

User.where(name: ["foo", "bar"])
# SELECT * FROM users WHERE name IN ('foo', 'bar')
Stefan
  • 109,145
  • 14
  • 143
  • 218
  • It's not just about quotes. %s appears to convert to a string whatever you throw at it and the result is placed as is in the query. No-no. Please also see my answer. – Giuseppe Oct 25 '13 at 15:02
  • @Giuseppe you're right, I've overlooked that the OP passes an array to `%s`. – Stefan Oct 25 '13 at 15:24
  • I guess the main takeaway here is that `%s` **doesn't** quote and therefore is not really too useful for sanitizing. It does, however, escape, so it's not useless either. `@dangerous` wouldn't necessarily have to be an array. It could be string of comma separated values, e.g. `@dangerous = "1, 2, 3"` – Alexander Clark Oct 28 '13 at 15:13
1

As far as I can tell, %s simply inserts into your query whatever @dangerous.to_s happens to be, and you are responsible for it.

For example, if @dangerous is an array of integers, then you will get an SQL error:

@dangerous = [1,2,3]
User.where("id IN (%s)", @dangerous)

will result in the following incorrect syntax:

SELECT `users`.* FROM `users` WHERE (id IN ([1, 2, 3]))

whereas:

User.where("id IN (?)", @dangerous)

produces the correct query:

SELECT `users`.* FROM `users` WHERE (id IN (1,2,3))

Thus, is seems to me that unless you know very, very well what you are doing, you should let the ? operator do its job, ESPECIALLY if you do not trust the content of @dangerous as safe.

Giuseppe
  • 5,188
  • 4
  • 40
  • 37
  • It seems that `%s` does escape, so it is "safe," but because it doesn't quote, it could have unexpected results, including errors. You are correct, probably best to avoid it unless one has a very good reason to do so. `Foo.where("bar = ?", @dangerous.to_s)` would accomplish the same thing, but with less worry about quoting correctly. But even this is probably unnecessary and maybe even undesirable most of the time. – Alexander Clark Oct 28 '13 at 15:18