「いちばん簡単な認証システム」のサンプルが安全ではなかった件

最初に断っておきますが、
いつも参考にしてる CakePHP のおいしい食べ方の
1年以上前のサンプルコードにセキュリティ上問題がある個所があって、
それを何とか中の人（dozono さん）に メールで伝えようと思って
いろいろ検索したり CakePHP の Google group に入ったりしたけど、
結局メールアドレスがわからななかったので、
blog にコメントしようかとも思ったけど、それにしては長くなるし
めんどくさくなって blog に書くことにしました。

07.12.18 追記

現在は修正されています。のでタイトルも変更しました。

本題。

CakePHP Controller の layout はあくまで表示を変えるためのメンバ変数である

問題あると思うのはこちらのコード

いちばん簡単な認証システム

class AppController extends Controller {


var $components = array(’SdAuth’);

function beforeFilter()

{

//for CAKE_ADMIN(admin) rooting.

if(defined(’CAKE_ADMIN’) && !empty($this->params[’admin’])){

//start admin check.

if($this->SdAuth->isloggedin() == FALSE)

{

$this->layout = “login”;

} else {

$this->layout = “admin_default”;

}

}

（以上、原文ママ）

多分、quote が全角になってるのは blog システムの都合だと思うので、スルーで。

このコードで何をしてるかというと、
SdAuth というコンポーネントを使って、
ログインしていなかったら ログイン画面を表示し($this->layout = “login”;)、
ログインしていたら管理画面用のレイアウトを表示する($this->layout = “admin_default”;)
という処理を行っています。

SdAuth という簡単な認証システムの性質上、
例えば以下のような使用を想定しているのではないかと思います。

• news という controller がある


• 管理者(ログイン時)のみが admin_add, admin_edit, admin_delete action を実行できる。
  url 的には 下記。 {n}はid の整数。未ログイン時はログイン画面を表示する。
  
  
  
  • /admin/news/add/
  
  
  • /admin/news/edit/{n}
  
  
  • /admin/news/delete/{b}
  
  


• 一般利用者は index, view actionのみが実行できる
  
  
  
  • /news/add/
  
  
  • /news/edit/{n}
  
  
  
  

ここで僕が問題視しているのは、Controller の layout 変数は
あくまで表示方法を変えるための変数なので、
layout 変数の変更によりログイン画面を表示しても、
リクエストされた action の処理は実行されてしまうということです。

つまり、下記 url にアクセスすると、
ログインの有無に関わらず id=1 の news を
削除できてしまいます。

/admin/news/delete/1

このサンプルコードを実装して debug モードでSQL を表示させつつ
admin しか実行できないはずの url にアクセスすると、
表示はログイン画面のまま、
処理が実行されているのがよくわかります。

対処法としては、下記のように、ログインしてなかったら
別途用意したログイン画面にリダイレクトしつつ、
ちゃんと exit してやる等が思い浮かびます。

            if($this->SdAuth->isloggedin() == FALSE) {

               $this->redirect('{ログイン画面のurl}');

               exit();

            } else {

                $this->layout = "admin_default";

            }

1年以上前の記事ではあるのですが、
「CakePHP acl」でgoogle検索すると割と上位に表示されるので、
修正された方がよいのではないかと思います。

07.12.18 追記

現在は修正されています。

というわけで、もっと CakePHP が盛り上がっていったらよいですね。

追記

少なくとも、 CakePHP1.2 で使用する場合には気を付けたほうがよいと思います。 CakePHP1.1 では試していないので、そっちでは大丈夫なのかもしれないです。

07.12.18 追記

以下のようにすれば変更が少なくてよいですね。

            if($this->SdAuth->isloggedin() == FALSE) {

                $this->layout = '';

                $this->render('../layouts/login');

                exit();

            } else {

                $this->layout = "admin_default";

            }