Hatena::ブログ(Diary)

ockeghem(徳丸浩)の日記 このページをアンテナに追加 RSSフィード

[PR]WAFの導入はHASHコンサルティング
 | 

2011-08-23

もし『よくわかるPHPの教科書』の著者が徳丸浩の『安全なWebアプリケーションの作り方』を読んだら もし『よくわかるPHPの教科書』の著者が徳丸浩の『安全なWebアプリケーションの作り方』を読んだらを含むブックマーク

 たにぐちまことさんの書かれた『よくわかるPHPの教科書(以下、「よくわかる」)』を購入してパラパラと見ていたら、セキュリティ上の問題がかなりあることに気がつきました。そこで、拙著「体系的に学ぶ 安全なWebアプリケーションの作り方(以下、徳丸本)」の章・節毎に照らし合わせて、「よくわかる」の脆弱性について報告します。主に、徳丸本の4章と5章を参照します。

4.2 入力処理とセキュリティ

 「よくわかる」のサンプルや解説では、入力値検証はほとんどしていません。しかし、入力値検証をしていないからといって即脆弱かというとそうではありません。徳丸本でも強調しているように、入力値検証はアプリケーション要件(仕様)に沿っていることを確認するもので、セキュリティ対策が目的ではないからです。

 「よくわかる」の中で、私が見た範囲で唯一の入力値検証は、郵便番号のチェックをするものです。以下に引用します(「よくわかる」P85)。

if (preg_match("/^\d{3}\-\d{4}$/", $zip)) {

 全体一致マッチの方法として「^」と「$」で挟むという方法をとっていますが、徳丸本P81で説明しているように、$はデータの終わりではなく、行末という意味なので、行末の\nがあってもマッチします。具体例を以下に示します。

var_dump(preg_match("/^\d{3}\-\d{4}$/", "111-1111\n"));
【実行結果】
int(1)

 このため、引用部分は以下のように書いた方がベターでしょう。

if (preg_match("/\\A\d{3}\-\d{4}\\z/", $zip)) {

 詳しくは徳丸本を参照ください。

 また、入力値検証ではありませんが、正規表現の使い方がおかしな箇所を見つけました。以下は「ひとこと掲示板」の投稿からURLを抜き出して、a要素のリンクを生成する関数です(P269)。

// 本文内のURLにリンクを設定します
function makeLink($value) {
    return mb_ereg_replace("(https?)(://[[:alnum:]\+\$\;\?\.%,!#~*/:@&=_-]+)", '<a href="\1\2">\1\2</a>' , $value);
}

 唐突にPOSIX文字クラス[:alnum:]が出てきますし、「よくわかる」の正規表現はpreg_matchで説明しているのに、この箇所はmb_eregを使っています。本書は文字エンコーディングとしてUTF-8を採用しているので、preg系の関数を使えないわけではなく、mb_eregが突然出てくる理由がわかりません。過去の資産からの使い回しでしょうか。「よくわかる」の本文(P270)には以下のように指摘されています。

正規表現の内容は理解するのはかなり難しいと思いますが、Googleで「正規表現 URL」などと検索をしたり「PHP URL リンク」などと検索すれば、サンプルプログラムを見つけることができるので、参考にしてもよいでしょう。

 現実問題として、多くのプログラマがこうやって、コピペプログラムを作っている現実があるにしても、もう少し注意点も書けばよかったですね。すぐに思いつく注意として以下があります。

 さて、先ほど引用したmakeLink()関数にはバグがあるようです。mb_eregの内部エンジンの鬼車は、Unicode文字コードの場合、[:alnum:]は半角英数字以外に、全角の英数字、平仮名、カタカナ、漢字などにもマッチします。これは恐らく意図した動作ではないでしょう。例えば、以下をご覧下さい。

var_dump(makeLink('例えばhttp://example.jp/を参照して下さい'));
【実行結果】
string(108) "例えば<a href="http://example.jp/を参照して下さい">http://example.jp/を参照して下さい</a>"

 全角の部分までリンクになっていますね。元のプログラムURLとして許される文字だけにマッチする正規表現と思われるので、これは意図した動作ではないと思われます。[:alnum:]ではなく、[a-zA-Z0-9]などと書けばよかった。

 これに近い注意は徳丸本のP83に、コラム「mb_eregの\dや\wに注意」として書いてありますので参照ください。

4.3.1 クロスサイト・スクリプティング(基本編)

 クロスサイト・スクリプティング(XSS)については、かなり気をつかっているように見受けられますが、それでももれはありました(P98のsample18.php)。これは絵に描いたようなXSSですね。

if (isset($_POST['my_id'])) {
    $_SESSION['my_id'] = $_POST['my_id'];
}
// 中略
<p>ようこそ<?php echo $_SESSION['my_id']; ?>さん</p>

 ちなみに、htmlspecialcharsの説明は、同書P47に既出です。

4.4.1 SQLインジェクション

 私が見た範囲ではSQLインジェクション脆弱性はありませんでしたが、気になる箇所はありました。

 まず、プレースホルダは用いておらず、mysql_real_escape_stringによるエスケープを採用しています。プレースホルダを使う方が望ましいと考えます(参考→Webアプリケーションとかの入門本みたいのを書く人への心からのお願い)。

 また、文字エンコーディングの指定に、以下を使っています。

mysql_query('SET NAMES UTF8');

 これはまずいとされていて、mysql_set_charset関数を使うべきです。詳しくは、千葉さんの素晴らしいエントリ「libmysqlclientを使うプログラムはset namesをutf8であっても使ってはいけない | へぼい日記」を参照下さい。

 もう一つ、数値型の扱いです。これは典型例で説明します(同書P253)。

$sql = sprintf('SELECT * FROM members WHERE id=%d',
    mysql_real_escape_string($_SESSION['id'])
);

 列idは数値型であり引用符で囲まれていません。これは正しいのですが、それにも関わらずmysql_real_escape_stringでエスケープしています。このエスケープは意味がありません。エスケープというのは、文字列リテラル中でのみ意味があるからです。このあたりの詳しくは、「安全なSQLの呼び出し方」を参照下さい。

 sprintfの書式%dを使っている時点で整数しか出力されない(すなわちSQLインジェクションは発生しない)ことが確実ですが、さらに整数へのキャストをするとよいでしょう。%d書式で受けるということは、整数型を期待しているわけですし、ソース上もSQLインジェクション対策が行われていることが一目でわかるようになり、チェック等が容易になります。

$sql = sprintf('SELECT * FROM members WHERE id=%d',
    (int)$_SESSION['id']);

4.5.1 クロスサイト・リクエストフォージェリ(CSRF

 「ひとこと掲示板」の投稿画面にCSRF脆弱性があります。

4.6.4 セッションIDの固定化

 ログイン時にセッションIDを変更していないので、セッションIDの固定化の脆弱性があります(同書P249)。

4.7 リダイレクト処理にまつわる脆弱性

 脆弱性ではありませんが、同書P89の以下の記述は誤りです。

引用者注:リダイレクトの)移動先のURLには、「http://」から始まる他のサイトのURLや「../」などから始まる相対パス、「/」から始まるルート相対パス」などが利用できます。

 HTTP/1.1の規格RFC261RFC2616では、Locationヘッダの指定するURLは絶対URLであることを要求しています(徳丸本P191脚注3)。

 また、狭義の脆弱性ではありませんが、リダイレクト後にただちに終了せず、後続の処理が動いている箇所があります。例えば、前述のログイン処理で、header関数実行後にexit();していないので、ログインフォームが出力されています(ブラウザには表示されません)。これはまぁバグですね。

追記(2011/09/09)

 ログインしていない場合にheader関数実行後終了せずに後続処理が動いている箇所があります。例えば、以下は、ログインしていない場合でも、投稿処理になだれ込んでいます。

} else {
    // ログインしていない
    header('Location: login.php');
}

// 投稿を記録する
if (!empty($_POST)) {
    if ($_POST['message'] != '') {
        $sql = sprintf('INSERT INTO posts SET member_id=%d, message="%s", reply_post_id=%d, created=NOW()',
// 後略

 こちらは、はっきりした脆弱性ですね。

4.8.1 クッキーの不適切な利用

 クッキーの不適切な利用がありますが、5.1.4自動ログインの項で説明します。

4.12.3 ファイルダウンロードによるクロスサイト・スクリプティング

 画像のアップローダがあり、XSSスクリプトインジェクションの問題を避けるために、拡張子をチェックしていますが、この処理にバグがあります。該当箇所(同書P110)を引用します。

$file = $_FILES['my_img'];
// 中略
$ext = substr($file['name'], -3);
if ($ext == 'gif' || $ext == 'jpg' || $ext == 'png') {
    $filePath = './user_img/' . $file['name'];
    move_uploaded_file($file['tmp_name'], $filePath);

 ご覧のようにファイル名の後ろ3文字を確認して、「gif」などであればOKとしていますが、この処理にバグがあります。拡張子がなくファイル名の末尾が「gif」などで終わっている場合(例:agifというファイル名)も許可されることになります。同様の問題が「ひとこと掲示板」にもあります(同書P239)。

 ファイル名に拡張子がなく、中味がHTMLである場合、IEではXSSが可能となります。詳しくは、徳丸本4.12節を参照下さい。

5.1.3 パスワードの保存方法

 パスワードSHA-1ハッシュの形で保存されています。以下にログイン用のSQLの組み立て部分です(P249)。

$sql = sprintf('SELECT * FROM members WHERE email="%s" AND password="%s"',
    mysql_real_escape_string($_POST['email']),
    sha1(mysql_real_escape_string($_POST['password']))
);

 2つの問題があります。まず、ソルトなしのSHA-1ハッシュでは、パスワード保護としてはあまり意味がありません。レインボーテーブルなどで元パスワードが解読できるからです。ソルトと、できればストレッチングをする必要があります。詳しくは、徳丸本P326以降を参照下さい。

 第2に実装が変です。先の実装では、パスワードSQL文字列としてエスケープしてからハッシュ値を求めていますが、まずハッシュ値を求めるべきです。sha1関数の結果は16進文字列ですからエスケープの必然性はありませんが、統一的にエスケープすることが望ましいでしょう。ということで、2つの関数の適用順序が逆です。

5.1.4 自動ログイン

 「ひとこと掲示板」には自動ログインの機能が用意されていますが、実装方法としてログインID(メールアドレス)とバスワードを平文でCookieに保存するという方法が用いられています(同書P250)。

 この方法は、徳丸本P330の「(自動ログインの)危険な実装例」として紹介されている方法です。また、「よくわかる」のP96には以下のように記述されています。

本文では、ログインIDのみをCookieに保存しました。Cookieには、いくつでも値を保存できるので、ログインパスワードも記憶できた方が、次回から自動的にログインできて便利になるでしょう。

しかし、Cookieブラウザの簡単な操作で内容を見ることができてしまううえ、これまでも悪意のあるプログラムによって、盗まれるといった被害が絶えず、安全に保管しておくことはできません。

そのため、Cookieには重要な情報を保存しておくべきではありません。【後略】

 引用部分に同意します。著者がこの方針を貫かなかったことが残念です。

5.1.7 ログアウト機能

 ログアウト処理では、GETメソッドを使っていることと、CSRF対策が抜けているところが気になったところですが、大きな問題ではありません。POSTメソッドを使った方がよい理由と、CSRF対策した方が良い理由は徳丸本を参照下さい。

5.2.1 ユーザ登録

 「ひとこと掲示板」には同一ログインID(メールアドレス)の重複確認をしていますが、この処理が不十分で、重複したアカウントができてしまいます。

 同書で採用している方法は以下の通りです。会員登録のフォームは、入力-確認-登録の3画面で構成されています。重複チェックは確認画面で処理されるので、ほぼ同時に同一ログインIDの登録が要求されると、重複のチェックができません。ほぼ同時といっても数秒程度のずれであれば、二重登録されるとみてよいでしょう。

 この問題は、徳丸本P347に「■ユーザIDの重複防止」として説明しています。また、同じページに「◆事例2:ユーザIDに一意制約をつけられないサイト」にも該当しています。「よくわかる」の場合は、一意制約をつけられない理由はなく、どうして一意制約をつけなかったのかは疑問です。

まとめ

 『よくわかるPHPの教科書』のセキュリティ上の問題について報告しました。今まで、PHPの書籍を何冊か読んできましたが、入門者がセキュリティも含めて学べる良書がないのが現状です。これまで読んだ本の中では、「PHP 逆引きレシピ (PROGRAMMER'S RECiPE)」と「パーフェクトPHP (PERFECT SERIES 3)」は比較的セキュリティに気を配って書いてありましたが、完全というわけではなく、また本当の入門者が学ぶのによいという性格の本でもなさそうです。

 このため、「よくわかる」などでPHPを学ばれた方は、次にはぜひ拙著「体系的に学ぶ 安全なWebアプリケーションの作り方」でセキュリティ面の強化をされるとよろしいかと思います(宣伝)。

 また、PHPアプリケーションセキュリティについては、PHPカンファレンス2011にて「徳丸本に学ぶ 安全なPHPアプリ開発の鉄則2011」という演題で講演することになりましたので、こちらもあわせてよろしくお願い致します。

[PR]

Kenji_sKenji_s 2011/08/23 09:58 そういえば、徳丸本 P.204 の redirect() 関数も exit() してないですね。

サンプルは redirect() して PHP ファイルが終了なので何のバグもありませんが。

defiantdefiant 2011/08/26 14:02 5.1.7 の部分 s/使っ他/使った/ でしょうか.:-)

ockeghemockeghem 2011/09/05 10:18 ありがとうございます。修正しました。

 | 
Google