Github に脆弱性。やった人は Rails に有りがちな脆弱性を issue に挙げていたが相手にされず、実際にそれを突いてきた。一見 childish だが、それだけ簡単に脆弱な実装がなされてしまうということだ。週明けの今日、Rubyist はまず関連情報に一読を。
— Yuki Nishijima (@yuki24) March 4, 2012
気になって調べたのでメモ。自分も気をつけないとなー。
- Public Key Security Vulnerability and Mitigation - github.com/blog/
github に脆弱性があってそれが突かれたらしい。 Rails アプリにありがちな脆弱性の一つ、Mass assignment とかいうタイプの脆弱性である。
mass assignment 脆弱性とは
mass assignment 脆弱性とは何か、というのは以下の記事が詳しい:
Rails 3.1 の新機能解説の記事だが、機能拡張の解説であって attr_accessible などは 3.1 より前からある。
要するに、モデルのメソッド update_attributes や new に Hash でデータを渡した時 attr_accessible (attr_protected) で指定した
カラムについては無視されるようにするという機能である。
update_attributes とは: ActiveRecord のモデルで、カラム名がキーとなった Hash を渡す事でデータを更新できるメソッド。foo.update_attributes(:title => "New Title") のように使います。
scaffold で生成されたコントローラの update アクションなどで使われていて、scaffold では update_attributes(params[:foo]) のようにパラメータが直接渡されています。
そして、update_attributes に HTTP のパラメータ params (Hash) を特にチェックせず直接渡して、
その params に本来更新されるべきではないカラムのパラメータが含まれていても更新されないようにするために attr_accessible, attr_protected をモデルで定義しておく事によって、update_attributes 等のメソッドにパラメータを渡しても、attr_accessible (attr_protected) で保護指定したカラムについては渡した Hash で指定されていても無視されるようになります。
上記のこの記事での例を引用すると、
attr_accessible :name とした事によって, ホワイトリスト方式で :name カラム以外は無視されるようになるため、
User.new で :group_id を指定していてもそれは無視されている事がわかるかと思います。
さて、この機能を使っておらず保護されていないカラムがあると何故脆弱性になるかはもう分かると思いますが、
たとえば実際にこのモデルを用いた Web アプリケーションがある時に、Web のインターフェースでは更新時に
update_attributes を特にパラメータをチェックせず直接 params を渡していて、
ユーザー (Web インターフェース) からは group_id を変更できないという事にします。
すると、 もし attr_accessible :name が無い場合 、Web インターフェースの update_attributes を呼んでいる所が、
Web インターフェースでは更新できるべきではない group_id を含んだパラメータを受け取ると 更新できるべきではない group_id が更新できてしまいます。
しかし attr_accessible :name がある事によって、update_attributes に :group_id を含んだ Hash (パラメータ) が来ても無視されるので変更される事は無いのです。
そのため、attr_accessible で明示的に「変更可能なカラム」を指定しておくのは重要で、これを指定していない事によって
更新されるべきではないカラムも更新することができる 脆弱性が mass assignment 脆弱性です。
しっかりホワイトリスト方式の attr_accessible で指定する事が一番安全でしょう。
update_attributes はいちいち user.name = params[:user][:name] 等としなくても Hash のキー・値でデータ (レコード)を更新できる
機能で便利なため、
update_attributes 以外にも new, create でもその保護は働くという事もあり、attr_accessible, attr_protected で保護するのが安全だと思います。
これは ActiveRecord を使ったアプリケーション (大半の Rails アプリケーション) は多く存在する可能性がありますが、 Mongoid でも同様の脆弱性が発生することがある そうです。
重要: これは Rails の脆弱性 ではなく Rails の問題 である。mass assignment 脆弱性に関しては Rails アプリケーション (もしくは ActiveRecord を使ったアプリケーション) において開発者が作りやすい脆弱性であるから。すぐ後述する Issue #5228 についたコメント (2段落目) にも書いてある。Rails の問題 というのは、現在の scaffold generator や ActiveRecord がこの問題を発生させやすいため、Issue 5228 では発生しにくくなるような対策を検討している。
今回の発端
さて、そこでこの github/rails/rails に登録された issue を見てみよう。今回の問題の発端である。
この issue で問題に上げているのは、現在の Rails は、開発者が mass assignment 脆弱性を発生させやすい (attr_accessible 等の存在とこのような脆弱性を知らないと)
ということ。
そのため、attr_accessible 等の指定を開発者に強制したりする等して、発生しにくくする案を探している。
(その後、いくつかの案が複数人によって挙げられているが今の議論、何が提案されているかまではこの記事では取り扱わない為、 issue ページを参照してください (それを見て、コメントするのも良いでしょう))
はじめは速攻で reject された。既にデフォルトで有効にする pull request があり (Pull Request #4062) それが reject されているからだ。
もちろん homakov さんは反論している。「ホワイトリストを必須にするのではなく、たとえばデフォルトで foreign_key, primary_key をブラックリスト入りさせておくのはどうか」 と。
(メモ: 一度は reject されましたが、現在は議論が再開されています)
脆弱性の問題を例示
そして、reject が繰り返されるとこのような issue が open される:
作成日時を示す部分を見て欲しい。"in 1001 years" とある。未来ですね。おそらく github が抱えている mass assignment 脆弱性をつついたのだろう。GitHub を使って例示してしまった。
たぶん created_at 等をパラメータに含ませて自分の指定した値に created_at を変更したのだろう。
そして彼はさらに別の所にある脆弱性もつつく:
一番最初に上げた github の blog の記事でメインに言及されている脆弱性がこれである。
At 8:49am Pacific Time this morning a GitHub user exploited a security vulnerability in the public key update form in order to add his public key to the rails organization. He was then able to push a new file to the project as a demonstration of this vulnerability.
(日本語訳: 8:49am (Pacific Time) に GitHub ユーザーは SSH 公開鍵の更新フォームに存在する脆弱性を攻撃し、彼の公開鍵は rails organization に登録された。そして彼は rails/rails プロジェクトに新しいファイルをコミットする事ができた、この脆弱性のデモとして。)
organization account も実は SSH 公開鍵を登録する事ができるので、たぶん SSH 公開鍵の更新のフォームで SSH 公開鍵のモデルにある user_id といったカラムを推測してパラメータに含ませ、pubkey を update したのだろう。どうやって account 側の primary key を取得したんだろう?
このように GitHub 等もうっかり mass assignment 脆弱性を抱えてしまう事から、デフォルトでいくつかの key をブラックリスト入りさせようと提案した訳である。まだこの issue は議論が続けられている: Issue #5228: Mass assignment vulnerability - how to force dev. define attr_accesible? · rails/rails
どのようにしてこの脆弱性の発生を防ぐか。現状だとユーザーが attr_accessible 等を手動で定義しないと、ユーザーがこの脆弱性の存在を知らない限り容易に発生させてしまうからね。rails だと。
GitHub のアクション
ちなみに今は解除されたが 彼の github アカウント は一時的に suspend された様である。何をしたかの調査、その間は害を及ぼした疑いがあるための suspend と見える。最初のアナウンス では単に suspend したとしか書いてないから誤解が多かったんだなぁ。Github, もちろん当初から疑いが晴れたら解除するつもりだったんだよね?
ちなみに誤解が多いと思われるのは そのアナウンス についたコメントでは「suspend を解除してやるべき」との声が多く上がっていた。
その suspend は実際に解除されたが、まぁ、解除すべきだよね。何か重大な損害を起こすような事はしてないものね。ただ、素直に報告せずに rails の所で実例を示してしまうというのはどうかと思うがw
まとめ
rails だと「気をつけましょう」になって、 php だと「これだから php は」になるっていう世にも恐ろしい何かを感じました！
— Keisuke SATO (@riaf) March 5, 2012
これだから rails は(キリッ, はやくなんとかしろ(キリッ
今後の動向が注目されますね。
追記 8: コントローラでみえる params に保護機能 ($SAFE な時の taint 的な感じで) をつける方向というのが DHH の考える一応の対応策? 別記事に詳細を書いた: rails/strong_parameters
あわせてよみたい: GitHub and Rails: You have let us all down. - Code Space
ところで、PHP は?
"register_globals？そんなのデフォルトでオフですよ" という記事より。
何年も前に指摘されていた脆弱性に今頃慌てふためいている愚かでコンピュータサイエンスを学ぶには無能すぎてRailsくらいしか使えないくせにPHPerだなんだと他人の尻馬に乗って調子こいてたボケナス共は、土下座してmodelの修正に取りかかるがよい。わっはっは。
だそうです。
「PHP: グローバル変数の登録機能の使用法 - Manual」… これはひどい。
あと、PHP の Web フレームワークの一つ Symfony にも似たような問題があるっぽい: Security must be taken seriously - Symfony (はてブコメントより)
はてブコメントを見て思ったこと
attr_accessibleはバリデータの一種なのかなぁ。- 自動生成で中身を把握してないというのも問題だが、これは自動生成じゃなくても
update_attributesとか普通に使うメソッドだし自動生成関係無くないかなあ。自分は今回この記事で自動生成 (=scaffold) を取り上げたけど。
追記メモ
追記: suspend は解除されていたようだ (実害を及ぼしたかどうかの疑惑が解けたから?) https://github.com/blog/1069-responsible-disclosure-policy ので、文章を一部修正
追記2: これは Rails の脆弱性と言うよりは Rails の問題点である事を追記。 (重要: 〜 の段落を追加)
追記3: セクションごとに見出しをつけました
追記4: register_globals の事を言及
追記5: 前提知識が必要だとの言葉を受けて若干追記。まだ分からない部分があれば twitter 等で指摘ください。
追記6: mass assignment 脆弱性の説明を大幅に書き換え。わかりやすくなったと思う。
追記7: PHP の Symfony にあった似たような脆弱性のつくりを紹介
追記8: strong_parameters の事を追記