Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

投稿削除の修正 #35

Open
marcy-terui opened this issue Apr 20, 2016 · 9 comments
Open

投稿削除の修正 #35

marcy-terui opened this issue Apr 20, 2016 · 9 comments

Comments

@marcy-terui
Copy link

ざっとソース読んで気づいたのですが、削除のキーが時刻になっているので同時刻の投稿があると意図せず消えてしまいます。
また、他もやるべきではあるのですが、ここはやろうと思えば任意の投稿を削除できてしまって非常にマズイと思うので、優先でCSRFの対策をすべきだと思います。

@n1215
Copy link

n1215 commented Apr 20, 2016

ログインして利用する機能が増えればCSRF対策も必要になりそうですが、この問題の根本はCSRFとは別ですね。
他サイトからのHTTPリクエストを封じても同サイト内からなら削除が可能です。

投稿を一意に特定すること、及び削除権限を示す何らかの情報が必要なので、
投稿時に削除用パスワード設定してIDをキーに削除するようにする、というのが実装コストの一番低い対策だと思います。
もしくは管理者のみ消せるようにするか。

@n1215
Copy link

n1215 commented Apr 20, 2016

過去のissueを見ると、投稿後5分以内は消せる仕様にする予定だったみたいですね。
こちらの修正ではクライアント側でボタンを非表示にする対応しか行っていないようです。
5分経過後も削除のHTTPリクエストが送れることに変わりはないので任意の投稿が消せる状態にあるのが現状。サーバ側で対応が必須ですね。
#22

@marcy-terui
Copy link
Author

ありがとうございます。
同じサイトからという話だと仕様的に5分以内?なら他人の投稿も消せてしまうので、とりあえず外側から消しまくるみたいな攻撃だけでも防げればと思いましたが、そもそもその前提がおかしいっていう話ですねw

投稿時に削除用パスワード設定してIDをキーに削除するようにする、というのが実装コストの一番低い対策だと思います。
もしくは管理者のみ消せるようにするか。

良いですね。削除用パスワードを控えそこねると消せなくなるので、できるなら両方あると良いですね。
(加えて削除依頼の問い合わせ先を用意する感じ)

@marcy-terui
Copy link
Author

@n1215 あ、言葉足らずですいません。一応削除処理にも5分のチェックは入ってました。「5分以内なら任意の投稿が消せる」という感じです。なので、ランダムに投げられまくるみたいなことをやられるとまずいなとCSRF対策を進めた感じでした。

@marcy-terui
Copy link
Author

ログイン機構は無くても、セッションだけ発行して削除パスワード(というかトークン)をセッション入れておいて、それに加えてCSRF対策をするのもありかなと思いましたが、ちょっと実装コスト高いですかね・・・

@n1215
Copy link

n1215 commented Apr 20, 2016

すみません、サーバ側も対応入ってたんですね、見逃してました。
おっしゃる通りセッション使って削除権限を暗黙的に扱うほうが、ユーザー側としては使いやすいです。要相談ですね。

@marcy-terui
Copy link
Author

#37 が入ったら以下の対応願います>誰となく

  • 削除は時間ではなくIDで行う
  • セッションに削除用トークンを発行(投稿にも保存して照合)して登録者のみ削除できるようにする

この2つができれば投稿者を外部で罠を踏ませる等しなければ攻撃は成立しないので、相当マシになります(逆に脆弱性は残されるということなので別途CSRF対策をしなくて良いということではない)

@marcy-terui
Copy link
Author

ブロックしていた #37 が解消したので進められるようになりました(結局 @sugumura さんと 自分でやったというw)

@n1215 n1215 mentioned this issue Apr 21, 2016
@n1215
Copy link

n1215 commented Apr 21, 2016

先にCSRF対策追加しました。メインの部分はどなたかお願いします。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants