idea * idea CakePHP修行 に重大なセキュリティホールが！

久しぶりのi d e a * i d e a CakePHP修行！ への返信です。

MAX_FILE_SIZEをhtmlヘルパーで書けないのか？

画像のアップロード処理をつくる【I】（CakePHP修行 #26） | i d e a * i d e aより。

さてまずはedit.htmlを変更したいのですが・・・MAX_FILE_SIZEの指定ではまりました。これってhtml helperでできないのかしらん？

<?= $html->hidden('MAX_FILE_SIZE', array('value'=>'30000')); ?>

現状では書けません。この話題についてはCakePHPでDBに関連しないパラメータはヘルパーで書くべきか？ (Re：CakePHP修行！ | idea*idea） : akiyan.comをどうぞ。

画像をアップロードするディレクトリのPermissionは777でよいのか？

画像のアップロード処理をつくる【I】（CakePHP修行 #26） | i d e a * i d e aより。

次にファイルアップ先の/webroot/picsを作り、Permissionを与えます（777にしたけどいいのかな？）。

アリともいえるしナシともいえます。ディレクトリのPermissionの問題は「上位のディレクトリの権限がどうなっているか」「グループ設定はどうなっているか」「サーバーを自分以外が使用するか」「セキュリティホールがあった場合、影響範囲をどこまでにおさえるか」などを考える必要がありますので、一概には言い切れません。

ポイントは自分以外の人間が正規の方法以外でそのディレクトリにファイルを作れてしまうかどうかです。これが無ければ、とりあえずはOKです。

しかし重大なセキュリティーホールが

しかし、このアップロード処理のコードには重大なセキュリティホールがあります。このセキュリティホールを利用すると任意のコードを実行できてしまいます。原因は、アップロードされたファイルの拡張子を変えずにDocumentRootの下に移動しているからです。

$ext = strtolower(preg_replace("!.*\.!", null, $this->data['User']['pic']['name']));
$filename = sprintf("%05d.%s",$this->User->id, $ext);
move_uploaded_file($this->data['User']['pic']['tmp_name'], APP."webroot/pics".DS.$filename);
$this->data['User']['pic'] = $filename;

攻撃用のコードを含んだ「attack.php」や「getcookie.html」といったファイルをアップロードされてしまった場合、phpであれば中身がそのまま実行され、htmlであれば埋め込まれたJavaScriptが動作してしまいます。要するに「何でもアップローダー」になってしまっているのですね。

このエントリを書いている時点の最新のコード（画像のアップロード処理をつくる【III】（CakePHP修行 #31） | i d e a * i d e a）ではファイルを移動せずにgd関数で画像を出力するようになりましたが、以前としてファイル名がそのままなので画像データに細工を施せば何らかのコードを埋め込むことができます。詳しくは画像へのPHPコマンド挿入 - T.Teradaの日記をどうぞ。

解決策の一つとして、アップロードされたファイルの画像形式を調べて、その形式にあった拡張子に変更してpicsディレクトリに配置することです。画像でなければアップロード処理はキャンセルしてしまいましょう。

ファイルというのは「ユーザーからのとても大きな入力値」ですので、普段の入力値と同じように入力値チェックやエスケープが必要です。今回の解決策として例示した拡張子の変更は、エスケープ処理のファイル版といえます。

皆さんもファイルのアップロード処理を実装する場合は、十分に注意して開発しましょう :)