日付の範囲検索を汎用的にしたら、SQLインジェクションってbrakemanに警告された話

日付の範囲検索

ActiveRecordのscopeを使って、created_atなどのdatetime型を範囲検索したくなりました。
前にqiitaで日付の範囲検索をするっていう記事があって、それを参考にモデルにscopeを書きました。

hoge.rb

 class Hoge < ApplicationRecord
   scope :created_at_between, lambda{ |from, to|
     if from.present? && to.present?
       where(created_at: from..to)
     elsif from.present?
       where('created_at >= ?', from)
     elsif to.present?
       where('created_at <= ?', to)
     end
   }
 end

これを他のモデルでも使いたいと思って、concernに書き出してみました。

between_scope.rb

module BetweenScope
  extend ActiveSupport::Concern

  def between_scope(*columns)
    columns.each do |column|
      scope "#{column}_between", lambda{ |from, to|
        if from.present? && to.present?
          where(column => from..to)
        elsif from.present?
          where("#{table_name}.#{column} >= ?", from)
        elsif to.present?
          where("#{table_name}.#{column} <= ?", to)
        end
      }
    end
  end
end

そして使いたいところでこんな風に使えます。

hoge.rb

 class Hoge < ApplicationRecord
   extend BetweenScope
   between_scope :created_at
 end

fuga.rb

 class Fuga < ApplicationRecord
   extend BetweenScope
   between_scope :created_at, :updated_at
 end

brakemanから警告

グッと使いやすくなって意気揚々とpushしたら、brakemanに怒られてしましました。

== Warnings ==

Confidence: Medium
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: where("#{table_name}.#{column} <= ?", to)
File: app/models/concerns/between_scope.rb
Line: 14

Confidence: Medium
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: where("#{table_name}.#{column} >= ?", from)
File: app/models/concerns/between_scope.rb
Line: 12

様々な対処法を探しましたが見つからず、、、
結局例外を吐くようにして、安全性を保つようにしました。

between_scope.rb

module BetweenScope
  extend ActiveSupport::Concern

  def between_scope(*columns)
    columns.each do |column|
      check_column column
      scope "#{column}_between", lambda{ |from, to|
        if from.present? && to.present?
          where(column => from..to)
        elsif from.present?
          where("#{table_name}.#{column} >= ?", from)
        elsif to.present?
          where("#{table_name}.#{column} <= ?", to)
        end
      }
    end
  end

  private

  def check_column(column)
    return if columns.select { |col| col.name.to_sym == column.to_sym && col.type.to_sym == :datetime }.present?
    raise ArgumentError, "Invalid argument: #{column}"
  end
end

こうしてもbrakemanの警告は出続けるので

bundle exec brakeman -I

で警告を無視するような対処をしました。

最後に

結果としてSQLインジェクションの危険性はなくなったと思ってますが、もっとスマートなやり方はないのかなと模索しています。
どこまでリスクを考えるべきなのか、線引きが難しいです。

参考記事

日付の範囲検索

brakemanの警告無視

SQLインジェクションを考慮しつつ、selectするカラムを動的に変更する