Blog

2月末に公表されていたRailsの潜在的な脆弱性の対策は?アンチパターンも載せとくよ。


脆弱性の内容は?

大垣さんや
https://www.facebook.com/yohgaki/posts/10153981283054452

徳丸さん
Ruby on Railsの潜在的なリモートスクリプトインジェクション脆弱性CVE-2016-2098
http://blog.tokumaru.org/2016/06/ruby-on-railscve-2016-2098.html

が指摘されたように、 Ruby on Rails に脆弱性が発見され、
対策されたバージョンが公開されています。

render params[:id]

など render に外部のリクエストパラメータを渡すと、任意の Rubyコードを実行できるというものです。


(暫定的な)対策は?

Rails のバージョンを
3.2.22.2, 4.1.14.2, 4.2.5.2
以上にあげましょう。

それで、render に任意のRubyコードが入力されても例外が発生して実行されません。


そもそも render params[:id] なんてやる奴おるの?

http://railsdoc.com/references/render
を見て分かる通り、色々な指定の仕方がありますが、ビューやテンプレートを指定するものです。

また、render をわざわざ指定しなくてもデフォルトのビューが描写されます。

show メソッドなら、 show.html.erb テンプレートが使用されます。

なのに、わざわざ標準的でない指定をする奴なんかいるでしょうか?

という事で Github で検索してみました。

Github Search

ごめんなさい。舐めてました。

全てが脆弱なコーディングかどうかは、分かりませんが、
リクエストパラメータを直接 render に渡す人がいる事が分かりました。

本当は、「render を不適切に使うやつなんかいないから、これは気にしなくていいよ」 という結論で終わらせたかったのですが、 ここからは、github で見つけたアンチパターンとその対策を載せて、啓蒙していこうと思います。


アンチパターン(テンプレートファイル切り替え)

Github で見ていて 明確に render params[:id] を使っているのは、
リクエストパラメータを使って、テンプレートを変えようというものでした。

workflow_lab3/app/controllers/pages_controller.rb

上記のコントローラーなんて分かりやすいですね。

/pages?id=views とかして、views テンプレートを使う想定なのでしょう。


設計を変えよう!

リクエストパラメータをそのまま使うのは、そもそもセキュリティプログラミング的にはNGです。

# 悪い例 bad example
class PagesController < ApplicationController
  def show
    case params[:id]
    when 'ui_xedit2'
      render params[:id], :layout => 'application2'
    else
      render params[:id]
    end
  end
end

このようにレイアウトをパラメーターで切り替えるのではなく できるのなら、

class PagesController < ApplicationController
  def show
  end

  def ui_xedit2
    render :layout => 'application2'
  end
end

とメソッド名を分けましょう。 これを有効にするには、config/rotues.rb

# 一例
# config/routes.rb

resources :pages do
  collection do
    get 'ui_xedit2'
  end
end

上記のような設定をすると

showメソッドでは、show.html.erb ui_xedit2メソッドでは、ui_xedit2.html.erb

でテンプレートを書けます。



もうひとつの方法

最後にもうひとつ、暫定的な方法で、いまいちな修正方法の紹介

# いまいち
class PagesController < ApplicationController
  def show
    case params[:id]
    when 'ui_xedit2'
      render "ui_xedit2", :layout => 'application2'
    else
      render :action => "show"
    end
  end
end

renderにリクエストパラメータを直接渡さないのがいいので、
条件分岐した後は、render にちゃんとSymbolや文字列で指定してあげる方法です。
良いといえばいいのですが、メンテナンスを考える時、
このshowメソッドを修正した時は、2つのテンプレートを同時に修正しないと
エラーが出る可能性がある所です。