Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Safely pass a dynamic column name into an ActiveRecord query with a Postgres cast?

I do a lot of time without date querying in my app, and I would like to abstract some of the queries away.

So say I have a model with a DateTime starts_at field:

Shift.where('starts_at::time > ?', '20:31:00.00')
-> SELECT "shifts".* FROM "shifts" WHERE (starts_at::time > '20:31:00.00')

This correctly returns all of the 'starts_at' values greater than the time 20:31.

I want to dynamically pass in the column name into the query, so I can do something like:

Shift.where('? > ?', "#{column_name}::time", '20:31:00.00').
-> SELECT "shifts".* FROM "shifts" WHERE ('starts_at::time' > '20:31:00.00')

In this example, this does not work as the search executes starts_at::time as a string, not as a column with the time cast.

How can I safely pass in column_name into a query with the ::time cast? While this will not accept user input, I would still like to ensure SQL injection is accounted for.

like image 201
chrismanderson Avatar asked Sep 27 '16 21:09

chrismanderson


1 Answers

This is more complicated than you might think at first because identifiers (column names, table names, ...) and values ('pancakes', 6, ...) are very different things in SQL that have different quoting rules and even quote characters (single quotes for strings, double quotes for identifiers in standard SQL, backticks for identifiers in MySQL, brackets for identifiers in SQL-Server, ...). If you think of identifiers like Ruby variable names and values like, well, literal Ruby values then you can start to see the difference.

When you say this:

where('? > ?', ...)

both placeholders will be treated as values (not identifiers) and quoted as such. Why is this? ActiveRecord has no way of knowing which ? should be an identifier (such as the created_at column name) and which should be a value (such as 20:31:00.00).

The database connection does have a method specifically for quoting column names though:

> puts ActiveRecord::Base.connection.quote_column_name('pancakes')
"pancakes"
=> nil

so you can say things like:

quoted_column = Shift.connection.quote_column_name(column_name)
Shift.where("#{quoted_name}::time > ?", '20:31:00.00')

This is a little unpleasant because we recoil (or at least we should) at using string interpolation to build SQL. However, quote_column_name will take care of anything dodgy or unsafe in column_name so this isn't actually dangerous.

You could also say:

quoted_column = "#{Shift.connection.quote_column_name(column_name)}::time"
Shift.where("#{quoted_name} > ?", '20:31:00.00')

if you didn't always need to cast the column name to a time. Or even:

clause = "#{Shift.connection.quote_column_name(column_name)}::time > ?"
Shift.where(clause, '20:31:00.00')

You could also use extract or one of the other date/time functions instead of a typecast but you'd still be left with the quoting problem and the somewhat cringeworthy quote_column_name call.

Another option would be to whitelist column_name so that only specific valid values would be allowed. Then you could throw the safe column_name right into the query:

if(!in_the_whitelist(column_name))
  # Throw a tantrum, hissy fit, or complain in your preferred fashion
end
Shift.where("#{column_name} > ?", '20:31:00.00')

This should be fine as long as you don't have any funky column names like "gotta have some breakfast" or similar things that always need to be properly quoted. You could even use Shift.column_names or Shift.columns to build your whitelist.

Using both a whitelist and then quote_column_name would probably be the safest but the quote_column_name method should be sufficient.

like image 67
mu is too short Avatar answered Nov 04 '22 20:11

mu is too short