Rails 5.2からRails SQL Injection ExamplesにあるようなSQLインジェクションを防ぐ仕組みが導入されて、Post.order(params[:order])
みたいなコードは心温まる正規表現によるチェックをパスしないと危険とみなされるようになって、お前が安全やと思うんやったらPost.order(Arel.sql(params[:order]))
しろってことになった(rails/rails#27947)。
これはRails 5.0のときにparams
がHashのサブクラスじゃなくなったときに比べればマシだけど、明らか安全やと思ってるリテラルもRailsに危険とみなされて既存のアプリケーションによったら非常にわずらわしい。たとえばDiscourseというRailsアプリは5.2に上げるときにこれの影響をモロに受けるんやけどっていうお気持ちを表明しています(rails/rails#32995 (comment))。
そこでこのわずらわしさを軽減する方法を考えたんですけど、もし#order
に渡される値がユーザーのリクエスト由来の未知の値でなくソースコード上に直に書かれた文字列リテラルだったとしたら、これは安全ってことにしていいと思いませんか?僕はそう思いました。
ソースコード上に直に書かれた文字列リテラルかどうかを完全に判別できる方法は今のところおそらく存在しないと思うけど、ユーザーのリクエスト由来じゃないかどうかをできるときだけできる範囲で区別するというのだと、# frozen_string_literal: true
のときにユーザーのリクエスト由来の値とちがって文字列リテラルはfrozenになるという性質が利用できそうです。という方法を実装したのが以下。
これにはひとつ物言いがついて、"#{user_input}"
がfrozenにならんのやったらええけどそこがなー的なこと言われてマジカーってなってたら、interpolated stringを判別する方法を教えてくれる神が降臨した(rails/rails#33330 (comment))。
# frozen_string_literal: true dynamic = DATA.read constant = "bar" interpolated = "#{dynamic} DESC" puts "[dynamic] frozen: #{dynamic.frozen?}, interned: #{dynamic.equal?(-dynamic.dup)}" puts "[constant] frozen: #{constant.frozen?}, interned: #{constant.equal?(-constant.dup)}" puts "[interpolated] frozen: #{interpolated.frozen?}, interned: #{interpolated.equal?(-interpolated.dup)}" __END__ foo
[dynamic] frozen: false, interned: false [constant] frozen: true, interned: true [interpolated] frozen: true, interned: false
パッと見なにを判別してるんかよくわからんかったので同僚のRubyコミッターに聞いたらプリントデバッグしながらいろいろ調べてくれた結果すべてを理解することに成功したのでその成果をここでシェアしたいと思います。
これはfrozen literalがRuby 2.5で導入されたfstring tableに登録されているものと同一かどうかを判別していて、constant literalだとfstring tableに登録されたインスタンスを返すけどinterpolated literalだとただのfrozenなだけのインスタンスになるという性質の違いを利用しています。String#-@
がfstringを返すのでconstant.equal?(-constant.dup)
はfstring tableからlookupしなおしたインスタンスが自分自身と同一だったら自分はfstringだったということでつまりconstant literalだったということがわかります。ちなみに.dup
が必要なのはRuby 2.5では自分がfrozenだったらString#-@
はfstringではなく自分をそのまま返してしまう問題があるからで、この問題はruby-headでは解決済みなので.dup
は要らなくなります(ruby/ruby@256411b)。
この方法は判別しようとするinterpolated literalのバリエーションだけfstring tableに登録してしまうので、なんか他にリーズナブルな方法ないかなって調べてくれたんですけど、今のところなさそう、ユースケース次第ですかねハハハーって感じでお開きとなりました。