「セキュアなPHPアプリケーションを作成するための7つの習慣」のサンプルがとんでもなく酷い

はてブで250以上のブックマークを得ている以下のエントリ。

PHP アプリケーションを作成する際には、可能な限りセキュアなアプリケーションにするために、次の 7 つの習慣を守る必要があります。

 ほう。しかし、内容はどうだろうか。
 読んでびっくりした。説明も微妙なところが多いが、サンプルが酷い。こんなサンプルでは悪い習慣が身についてしまう。全部は書ききれないと思うので、目についたところからピックアップして紹介する。

パストラバーサルのサンプル

 説明自体はまずまずなのだが、元の脆弱なサンプルが以下のようになっている。

    $file = $_POST['fileName'];
    //中略
    $fh = fopen($file, 'r');
    while (! feof($fh))
    {
        echo(fread($fh, 1024));
    }
    fclose($fh);

 説明では、ファイル名をホワイトリスト(英数字とピリオドのみを許し、かつ連続するピリオドを禁止する正規表現)でチェックする改善案を示している。
 しかし、それでよいとすると、元のファイルはPHPと同じディレクトリに格納されていることになる。すなわち、ファイルは外部から公開された状態になるのだ。であれば、いちいちファイル処理をするまでもなく、リンクを示せばいいだけではないのか。いや、それだと「Content-Disposition: attachment; filename=」というレスポンスヘッダが指定できないわけだが、アクセス制御の問題の要否など、サンプルとしても隙が多すぎる。

データベースを保護する

 ここでは、SQLインジェクション対策が説明されているわけだが、ここでもサンプルが酷い。
 対策後のコードを一部引用する。

function isValidAccountNumber($number) 
{
    return is_numeric($number);
}

if ($_POST['submit'] == 'Save') {
    /* Remember habit #1--validate your data! */
    if (isset($_POST['account_number']) &&
    isValidAccountNumber($_POST['account_number'])) {
    // 中略
        $select = sprintf("SELECT account_number, name, address " .
	                  " FROM account_data WHERE account_number = %s;",
                      mysql_real_escape_string($_POST['account_number']));

account_numberは数値なので、関数isValidAccountNumber内で数値としての妥当性チェックをした後、SQLを組み立てる際に、mysql_real_escape_stringでエスケープしている。しかし、エスケープするのであれば、SQLにおいては、account_numberをシングルクオートで囲ってやらなければならない。数値文字列をエスケープ処理して、クオートしたものをSQLとして用いる方法は大垣靖男氏が提唱されている方法で、私はこの方法に賛成ではない(変数に型のない言語におけるSQLインジェクション対策に対する考察(5): 数値項目に対するSQLインジェクション対策のまとめ - 徳丸浩の日記(2007-09-24)参照)のだが、きちんと動作はする。しかるに、クオート抜きでエスケープのみするのでは、何の意味もない。
 もう少し説明しよう。大垣メソッドは以下のような考え方だ。数値パラメータはバリデーションで数値としての妥当性チェックをしておくのだが、万一検証が漏れた場合のことを考えて、以下のように文字列化してSQLを組み立てる。

SELECT account_number, name, address FROM account_data WHERE account_number = '123'

あらかじめパラメータをSQLエスケープしておくことで、入力検証をすり抜けた場合でも、SQLインジェクション対策になるというものだ。

「1 OR 1=1」を入力とした場合
SELECT account_number, name, address FROM account_data WHERE account_number = '1 OR 1=1'

「'OR'a'='a」を入力とした場合
SELECT account_number, name, address FROM account_data WHERE account_number = '''or''a''=''a'

いずれも、SQLインジェクションにはならない。
 しかし、先ほどのサンプルではシングルクォートで囲っていないために以下のような攻撃が可能となる。

「1 OR 1=1」を入力とした場合
SELECT account_number, name, address FROM account_data WHERE account_number = 1 OR 1=1

「'OR '='」を入力とした場合
SELECT account_number, name, address FROM account_data WHERE account_number = ''or''=''

慣れない方には見えづらいかもしれないが、どちらもWHERE句が無効にされている(全件参照)。すなわち、シングルクォートで囲まないパラメータをエスケープしても意味がないのだ。

 ついでながら、エラー処理もおかしい。

$link = mysql_connect('hostname', 'user', 'password') or
die ('Could not connect' . mysql_error());
...
$result = mysql_query($select) or die('<p>' . mysql_error() . '</p>');

 これだとMySQLのエラーメッセージが画面表示されて攻撃者へのヒントになりこそすれ、正規ユーザにはちっとも親切ではない。しかも、これは確信犯だ。mysql_query側のdie呼び出しにはpタグまでつけているのだから。
 こういうところだけみても、筆者はWebアプリのセキュリティに関しては素人ではないかと疑わざるを得ない。 

XSS脆弱性から保護する

XSS対策としては以下のコードが例示されているのだが、

echo(htmlentities($_POST['myText']));

htmlentitiesに対するオプショナルなパラメータが与えられていないので、様々な文字エンコーディングに対して正しく処理できない。

まだあるかもしれないが、もう寝ようと思っていたところにこの記事を見かけたので、ついカッとなってここまで書き上げた。もう十分だろう。元記事の筆者はWebアプリケーションの脆弱性についての専門家ではないようだ。くれぐれも中身のコードを真似されないように。