PHPで最低限注意すべきな危険な臭いのする箇所

よく、○○攻撃とか○○脆弱性とかといった、セキュリティ的にまずい部分について攻撃の手法などでまとめることがありますが、今回はコードベースで危険を感じ取れるようなまとめ方にしたいと思います。

PHPで、とあるのは例にあげるコードがPHPだからなのですが、おそらく大体の部分は他の言語でも通用するかと思います。また、ここで挙げるものが注意すべき事項全てではないことをご承知ください。。

フレームワークを使えばかなり防げてしまうような単純な例ばかりですが、参考になればと思います。

1 画面表示関係
2 データベースへのアクセス
3 ファイルアップロード
4 GETでのデータ操作
5 ファイル処理
6 リクエスト変数のグローバル化
7 header()関数
8 インクルード
9 eval関連
10 外部コマンド実行
11 まとめ

画面表示関係

これがいちばん基本的なところだと思います。

printやechoなどでユーザデータを画面に表示する箇所です。パターンとしては、大きく分けて以下の2つでしょうか。

$_GET,$_POST,$_REQUEST,$_SERVER変数の表示
データソースからの値の表示

例えば、以下のようなコードは危険な臭いがします。

<?php
    print $_GET["name"]; // GETからのデータを表示
    print $record["address"]; // データベースに入っているデータを表示
?>

htmlspecialchars()関数で処理してから表示すべきですが、こういった処理はテンプレートエンジンに任せてしまったほうが良いと思います。

とにかく処理後表示させればOKという考え方でいろんな箇所でhtmlspecialchars()をとりあえず実行して変換しているコードを見たことがありますが、表示の直前に一元的な方法で処理を行わないと抜けが出たり2重に処理してしまったりといったケアレスミスをしがちです。

データベースへのアクセス

これもよくある問題です。いわゆるSQL Injectionという攻撃で、これを受けると不正にデータを操作されてしまったり不正ログインを許してしまったりすることがあります。

注意すべき点としては、

ユーザデータを利用したSQL文の構築
ユーザデータをそのままフィールド名やテーブル名、予約語として利用

の2つがあります。

前者は例えば、

$sql = "SELECT * FROM user WHERE loginid = '".$_POST["loginid"]."' AND passwd = '".$_POST["passwd"]."'";
$result = query_exec($sql); // query_exec()はSQLの実行関数とする

というもので、passwdにクォート文字や任意のSQLを入れることで想定外のクエリを実行できてしまいます。この場合の問題はPOSTから来た値がエスケープされていないことで、RDMSに対応したエスケープ関数で前処理しておけば防ぐことができます。

望ましいのはadodbやPDOなどで、値についてはプレースホルダで渡してやる方法だと思います。コード自体がRDMSの種類への依存度が下がるため結合がゆるくなるというメリットもあります。

後者の場合は、

$sql = "SELECT * FROM user WHERE is_delete = false ORDER BY ".$_GET["column"]; // ソート順の切り替え
$result = query_exec($sql);

のような場合で、この場合はcolumnにUNIONで別のSELECT文とつなげたり、セミコロンを使えば任意のSQLが実行できたりしてしまいます。

ソートキーなどをユーザ操作で変更したい場合などは、ユーザからのデータは数値で受け取り、その値に応じて定数定義しておいたフィールド名を使うようにし、直接フィールド名を受け取るべきではありません。

ファイルアップロード

ファイルアップロードの処理はセキュリティだけでなく、色々と悩ましい部分です...。

セキュリティの部分でいえば、以下のようなコードがあれば注意すべきでしょうか。

$_FILESのtypeの値を信頼してしまっている
拡張子のチェックを行っていない
swfファイルのアップロードを許可している
ドキュメントルート以下の公開領域へアップロードファイルを保存している

まず、$_FILESのtypeの値は単純にリクエストヘッダの値がそのまま入っているので、全く信用できません（クライアント側でいくらでも操作できるため）。この値を使うのではなく、ファイルの内容自体で検証すべきですが、難しい場合もあるので拡張子チェックは最低限行うべきだと思います。画像の場合はGD関数などである程度検証できます。

swfファイルのアップロードは、それ自体問題があるわけではありませんが自動的にそれが表示されるような仕組みの場合は大きな問題となります。swfファイルはスクリプトの塊みたいなものなので、JavaScriptを自由にアップロードできるのと同じくらいかそれ以上の危険性があります。

公開されている領域にアップロードすることは、画像などの形式の制限がされていれば問題は起こりませんが、例えば .php ファイルなどがアップロードできる場合、任意のスクリプトが実行できてしまい大変大きな問題となりえます。

GETでのデータ操作

例えば、以下のようなHTMLがあってリンクをクリックして削除操作ができることを想像します。

<a href="delete.php?id=1">削除</a>

この場合、例えば外部のサーバやメールなどに上記URLを使ったリンクでHTMLを作成し、上記の削除権限を持ったユーザにそのリンクを踏ませると意図しないところで削除されてしまいます。Javascriptなどと併用すると、全く気付かないうちに上記URLへリクエストを送っているということも可能となります。

この対策としては、POSTでのみ処理を行うようにするかHTTP_REFERERによる規制を行うという方法があります。POSTでの制限は、HTTP_REFERERの制限に比べて弱く、Javascriptを使うと回避する方法があります。

他に、トランザクショントークンという変数を使って各ページごとにランダムで1回のみ有効な値を発行し、画面の遷移に対して制約をかける方法もあります。強力ですが、ブラウザの戻るボタンが使えなくなったり導入についての工数が多くなったりと問題もあります。

ファイル処理

これは、アップロードとはちょっと異なり、ファイルシステムの操作を行う場合のことです。

fopen(),file(),copy()などといったファイルシステム関数を使う場合で引数にユーザデータを取る場合、気をつけないと意図しないファイルを処理できてしまいます。

$data = file("/var/www/html/uploads/" . $_POST["filename"] . ".dat");

というコードの場合、"../../../../" と相対パスで記述した値を渡すことでどんなパスのファイルでも指定できてしまいます。

拡張子が .dat に制限できていると思われますが、ヌルバイト攻撃と呼ばれる方法で無効にできてしまいます。これを防ぐためには、あらかじめ .. がないかどうかのチェックと、ヌルバイトの削除をしておく必要があります。

可能ならば、ファイルシステム上の実ファイル名などはユーザデータに依存しない方法で扱うべきです。

リクエスト変数のグローバル化

これは、register_globalsに関連する問題で、たとえばもともとregister_globalsがオンである前提で書かれたコードをオフの環境で動かすために以下のようなコードを使っている場合です。

foreach($_GET as $key => $value)
{
    $$key = $value;
}

これは、たとえばGETパラメータとして _SESSION[変数名]=値のようなものを与えるとセッション変数の上書きができてしまいます。

新しく開発する場合であれば、register_globalsをオフにして上記のような変数の作成をやらなければ良いのですが、古いアプリケーションを動作させるようにする場合はなかなか対応が難しい問題です。簡単な方法は、変数名のブラックリストを作ってそれに該当するものは代入しないようにすればよいのですが、ブラックリストに入っていないものは抜けが出てしまうという問題もあります。

header()関数

header()関数は、リダイレクトの処理やファイルダウンロードなどでよく用いられますが、ここの引数にユーザデータを入れる場合は注意が必要です。

というのも、ユーザデータ内に改行コードを入れることで任意のレスポンスヘッダを作成できてしまうからです。これによってクッキーを自由に操作したり任意のページへリダイレクトさせたりできてしまいます。

対応は簡単で、改行コードを削除すればOKです。

インクルード

includeやrequireにユーザデータを含める場合は注意が必要です。

include $_GET["include"];

こんなコードを書く人はまさか居ないとは思いますが、こういうコードがあるとサーバ上の任意のコードばかりでなく、リモート上の任意のコードも実行できてしまう可能性があります。

非常に危険なので、includeやrequireは固定値を取るようにした方がいいと思います。

eval関連

evalというのは文字列データを命令として実行する関数のことですが、これにユーザデータを含ませると・・明らかに危険な臭いがしますね。。

場合によっては任意のPHPコードが実行できてしまうので、eval関連は使わないほうがいいように思います。evalでなきゃだめなケースってほとんどない上に、可読性も悪くなるので・・。

外部コマンド実行

system()やexec()などを使っている箇所にユーザデータを渡している場合は要注意です。パイプなどを組み合わせると任意のコマンドが実行できてしまいます。

escapeshellarg()で引数についてはエスケープできますが、なるべくコマンド実行系は使わないほうがいいと思います。

まとめ

以上、ポイントをまとめるとだいたい次のような感じでしょうか。

リクエストデータはあらかじめNULLバイトを削除する
画面表示時はhtmlspecialchars()などで処理する
データベースへのアクセス時、値はadodbなどでプレースホルダを使う
ユーザデータをそのままフィールド名などに使用しない
$_FILESのtype値は信用できない。拡張子かデータ自体で検証する
swfファイルはアップロードを許可しない
重要なアクションはPOSTを使い、リファラ制限またはトークンを使う
ファイル処理はユーザデータをそのまま使用しないファイル名を使う
register_globalsはオフにする
header()関数で使うデータは改行コードを削除する
includeやrequireではユーザデータは使わず固定のファイル名を使う
evalはなるべく使用しない
外部コマンド実行はなるべく使用しない

他にも色々あると思いますが、最低限これくらいは考慮しときたいなぁと思います。

Pocket