Project

General

Profile

Code Review #965

Mass assignment security vulnerability

Added by Haru Iida over 7 years ago. Updated over 7 years ago.

Status:
終了(Closed)
Priority:
高め(High)
Assignee:
Target version:
-
Start date:
03/13/2012
Due date:
% Done:

70%

Estimated time:
Code review: 52:/app/controllers/issue_templates_controller.rb@8b44672b6382:line 32

Description

私の元のコードが悪いのですが、Mass assignment脆弱性があります。

@issue_template.attributes = params[:issue_template]

で値を設定すると、HTTPリクエストのパラメータにproject_idやauthor_idを勝手に追加して意図しないフィールドの値を書き換えられてしまいます。

修正方法としては、モデル側でRedmine::SafeAttributesをインクルードして書き換えてもよいフィールドを定義し、コントローラ側でattributes = の代わりにsafe_attributes =で値を設定すればよいと思います。以下の修正を参考にして下さい。

http://www.redmine.org/projects/redmine/repository/revisions/9140

Associated revisions

Revision 3:e05b4de5ba1b (diff)
Added by Akiko Takano over 7 years ago

Prevent mass-assignment when adding/updating. (Follow the way of Redmine itself. IssueID: #965)

Revision 4 (diff)
Added by Akiko Takano over 7 years ago

Prevent mass-assignment when adding/updating. (Follow the way of Redmine itself. IssueID: #965)

Revision 4:b2a8431dfb76 (diff)
Added by Akiko Takano over 7 years ago

Modify test code. (Refs: #965)

Revision 5 (diff)
Added by Akiko Takano over 7 years ago

Modify test code. (Refs: #965)

History

#1

Updated by Akiko Takano over 7 years ago

  • Status changed from 新規(New) to 担当(Assigned)
  • Priority changed from 通常(Normal) to 高め(High)

ありがとうございます。
なるほど、全部書き換えできてしまうんですね。#なによりもお恥ずかしい...。

そして、ポイントも大変助かりました、ありがとうございます!

パラメータinjection(?)に対してもテストコードを書いたりするのがやはり定石なんでしょうか。

#2

Updated by Akiko Takano over 7 years ago

  • Status changed from 担当(Assigned) to 解決(Resolved)
  • % Done changed from 0 to 70

対応前の動作確認で、確かに値の変更ができてしまいましたので、commite: 05b4de5ba1b で対応してみました。
safe_attributeに変更しただけですが...。

issue_template_settingsも同様の修正をしています。
一応テストコードで、ProjectIDとAuthorIDをパラメータで渡しても上書きされないことは確認してみました。

#3

Updated by Akiko Takano over 7 years ago

  • Status changed from 解決(Resolved) to 終了(Closed)

Closed.

Also available in: Atom PDF