Brakeman sql injection warning for string interpolation

229 Views Asked by At

I ran brakeman on my software and it has came back with some sql injection warnings. Most of these consist of lines of code that contain string interpolation. Is this is false positive or is there different syntax that can be used to avoid these warnings?

An example of a warning I have is:

self.by_searchstr(search).order(arel_table[:id]).joins(:cow).joins(:gender_lookup).order("#{sort_order}")

'Message: Possible SQL injection'

It specifically highlights 'sort_order'

I would hope there to be a simple fix for this as I have plenty others exactly like this.

1

There are 1 best solutions below

2
max On

order can be called with a list of symbols:

order(:foo, :bar, :baz)

You can compose and whitelist this list as an array and use the splat operator to convert arrays into a list:

order(*whitelisted_columns)

Or a hash where the keys are the columns and the values are the direction:

order(foo: :asc, bar: :desc, baz: :asc)

This isn't an actual keyword argument so you don't need to use a double splat if you call it with a pre-composed hash.

orderings = { foo: :asc, bar: :desc, baz: :asc }.slice(*whitelist)
order(orderings)

You can also use a combination of both list and hash arguments.

order(:foo, { bar: :asc, baz: :desc})

If you are taking this as user input you should whitelist both the names of the columns and the direction.

Calling order with a SQL string or Arel should only be done if you completely trust the inputs and if the job actually requires it.