diff options
| author | Teddy Wing | 2018-03-05 17:39:19 +0100 | 
|---|---|---|
| committer | Johan Van Ryseghem | 2018-03-07 13:19:41 +0100 | 
| commit | 7034809b7ce458c32597026d69afdf5c37db6bd4 (patch) | |
| tree | 6cf5863b2de2e84baaef12e8c673eced20dab2b5 | |
| parent | 3dea399922f40eb6afeca9f08910ab91cddeefea (diff) | |
| download | chouette-core-7034809b7ce458c32597026d69afdf5c37db6bd4.tar.bz2 | |
AutocompleteLines: Sanitize `:q` param in `LIKE` operator
Johan made a number of good points here:
> * I think this belongs in the model
> * I would rather use a named parameter here
>   `.where('lines.number LIKE :q OR lines.names LIKE :q ...', q: >   "%#{params[:q]}%")`
> * You should defiitely escape the params before passing it to your db.
>   `sanitize_sql_like` seems like the best choice here
I wasn't thinking about sanitisation at all and just assumed the `?`s in
the prepared statement would take care of it for me. But obviously,
we're passing `%`s in the param, so users can of course do the same
thing.
Protect against this using the
`ActiveRecord::Sanitization#sanitize_sql_like` method. This is a private
class method, so in order to use it we have to call it from inside the
`Chouette::Line` model.
And of course the named parameters are a no-brainer. At the time, I had
seen that `Array` splat somewhere else in the codebase and just blindly
copied the format, forgetting that named parameters even existed.
Refs #5889
| -rw-r--r-- | app/controllers/autocomplete_lines_controller.rb | 8 | ||||
| -rw-r--r-- | app/models/chouette/line.rb | 10 | 
2 files changed, 11 insertions, 7 deletions
| diff --git a/app/controllers/autocomplete_lines_controller.rb b/app/controllers/autocomplete_lines_controller.rb index 6f5009390..8398a92c1 100644 --- a/app/controllers/autocomplete_lines_controller.rb +++ b/app/controllers/autocomplete_lines_controller.rb @@ -9,13 +9,7 @@ class AutocompleteLinesController < ChouetteController      @lines = referential.line_referential.lines      @lines = @lines -      .joins(:company) -      .where(' -        lines.number LIKE ? -        OR lines.name LIKE ? -        OR companies.name ILIKE ?', -        *Array.new(3, "%#{params[:q]}%") -      ) +      .by_name(params[:q])        .search(params[:q])        .result        .paginate(page: params[:page]) diff --git a/app/models/chouette/line.rb b/app/models/chouette/line.rb index b3c4f2463..f65d313b3 100644 --- a/app/models/chouette/line.rb +++ b/app/models/chouette/line.rb @@ -43,6 +43,16 @@ module Chouette      scope :by_text, ->(text) { where('lower(name) LIKE :t or lower(published_name) LIKE :t or lower(objectid) LIKE :t or lower(comment) LIKE :t or lower(number) LIKE :t',        t: "%#{text.downcase}%") } +    scope :by_name, ->(name) { +      joins(:company) +        .where(' +          lines.number LIKE :q +          OR lines.name LIKE :q +          OR companies.name ILIKE :q', +          q: "%#{sanitize_sql_like(name)}%" +        ) +    } +      def self.nullable_attributes        [:published_name, :number, :comment, :url, :color, :text_color, :stable_id]      end | 
