最初、書いた時はユーザーが一人だけと頭にあったので思いっきり誤解していました。確かに複数ユーザーの場合は真正性に問題があります。修正版の差分をアーカイブを更新しておきました。
本題のブログはこちらです。
書籍『Webアプリケーションセキュリティ対策入門』のCSRF脆弱性
トークンの有効範囲は?
トークンがDBに保存される場合、トークンの有効範囲が気になるところです。大垣本および第二版のソースを見ると、トークンを保存するテーブルの定義は以下の通りです。
sha1がトークン、createdが生成日時を保持します。
シンプルな構造ですが、これだとトークンは、ユーザーやセッションを超えて、アプリケーション全体で共通になっています。これはまずそうですね。
トークンをホテルの部屋の鍵に例えると、こうです。大垣方式の鍵は、ホテルの全ての部屋に共通で使える鍵です。本来は、鍵は特定の1部屋のみに使えるべきですが、そうなっていないのです
複数ユーザーの場合、セッションIDが他のユーザーに見えると困ることと同じでユーザーまたはセッション毎に紐づけしないと不正な操作を外部からされる可能性があります。
脆弱だ、とされているプログラムはこちらです。
修正した差分はこちらです。
0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 | --- a/sample-bbs/lib/bbs.class.php +++ b/sample-bbs/lib/bbs.class.php @@ -184,7 +184,8 @@ class BBS { ), 'form_id' => array( - array('post', 'sha1'), + array('post', 'str'), + array('post', 'str_length', '', 0, 81), ) ), 'errors' => @@ -357,12 +358,16 @@ class BBS { } public function CheckFormID($sha1, $disable=true) { + $this->db->Begin(); $form_id = $this->db->GetFormID($sha1); + var_dump($form_id, $sha1); if (!$form_id || $form_id !== $sha1) { $GLOBALS['error']='送信済みです'; + $this->db->Commit(); return false; } else { $this->db->RemoveFormID($sha1); + $this->db->Commit(); } return true; } diff --git a/sample-bbs/lib/database.class.php b/sample-bbs/lib/database.class.php index dd3b02c..aff6e09 100644 --- a/sample-bbs/lib/database.class.php +++ b/sample-bbs/lib/database.class.php @@ -180,12 +180,23 @@ class BBS_DB_Main extends DB { } } + public function Begin() { + $this->Exec('BEGIN EXCLUSIVE TRANSACTION;'); + } + + public function Commit() { + $this->Exec('COMMIT TRANSACTION;'); + } + // FORM_IDを取得 public function CreateFormID() { + if (!session_id()) { + trigger_error('セッションがありません'); + } // /dev/urandomなどのセキュアなRNG（乱数生成機)を利用した方が良い // $this->secretが安全ならこれでも安全 - $sha1 = sha1($this->secret . mt_rand() . microtime()); - $sql = "INSERT INTO form_id (sha1, created) VALUES (". $this->quote($sha1) .", ". $this->quote(time()) .");"; + $sha1 = sha1($this->secret . mt_rand() . microtime()).'-'.sha1(session_id()); + $sql = "INSERT INTO form_id (sha1, created) VALUES (". $this->quote($sha1).", ". $this->quote(time()) .");"; if (!$this->Exec($sql)) { trigger_error('FORM_IDの作成に失敗しました。'); } @@ -194,8 +205,14 @@ class BBS_DB_Main extends DB { // FORM_IDを検索 public function GetFormID($sha1) { - $sql = "SELECT * FROM form_id WHERE sha1 = ". $this->quote($sha1) .";"; + $tmp = preg_split('/-/', $sha1); + if ($tmp[1] != sha1(session_id())) { + trigger_error('セッションIDが無効です。'); + } + $this->Begin(); + $sql = "SELECT * FROM form_id WHERE sha1 = ". $this->quote($sha1).";"; $stmt = $this->Query($sql); + $this->Commit(); if (!is_object($stmt)) { trigger_error('データベースエラーが発生しました'); } |
EXCLUSIVE TRANSACTIONでトランザクションをシリアライズすることと、$form_idにハッシュ化したセッションIDを付けてユーザーに紐づけするようになっています。
これで複数ユーザーになった場合でも、他のユーザーが他のセッションにCSRFトークンを利用できなくなると同時に、トランザクションでレースコンディションを防止できます。１つ問題なのはセッションIDが切り替わった場合です。これに対処するには$_SESSIONに最後のIDを保存しておいて、現在のIDと違う場合に前のIDを使うように変更すると対応できます。
使い捨てのCSRFトークンは、固定または半固定のCSRFトークンよりかなり安全ですが問題もあります。使い捨てのCSRFトークンはフォームのみでなく、大量データの一覧にも使うことができます。セッションに使い捨てのCSRFトークンを保存すると、セッションデータが大きくなりすぎてパフォーマンスに影響を与えてしいます。GCも性能に影響を与えます。
使い捨てのトークンを実装する場合、memcachedなどの自動的に有効期限が切れるデータベースを使うと便利かつ良い性能を期待できます。memcachedを利用する場合、$form_id . “.lock”などのロックレコードを使ってロックしないと、レースになる場合があります。
時々やってしまうのですが、大きな勘違いをしていました。サンプルとして大きな欠陥でした。時間が無いから、といって斜め読みすると危険です（汗
認証系のライブラリなどでも今回間違えていたような、この種類の前提ミスがあることが時々あります。他山の石として注意してください。
指摘頂いた徳丸さん、ありがとうございました。また、最初に投稿したときは誤解していて失礼致しました。