外部ファイルからのGETパラメータ受け取り時のセキュリティについて

Question

外部のファイルからリンク先のファイルにパラメータを渡し、リンク先のページでそのパラメータ値を受け取る際のセキュリティについて不安なので投稿させてもらいました。

例えばindex.phpに、
<a href="foo.php?category=1">カテゴリー1</a>というリンクがあった場合、
foo.phpでは$_GET["category"]に1が入りますが、URLパラメータ改ざんの恐れがあるためfoo.phpで

if(!empty($_GET["category"]) && preg_match("/1/u", $_GET["category"])){
$category_no = (int)$_GET["category"];
}else[
die("error");
}

としました。
この他にチェックすべき点はありますでょうか。 また上記のコード自体、意味が無いもので別の書き方がありますでしょうか。

よろしくお願いします。

Answer 1

preg_match("/1/u", $_GET["category"])だと1が含まれていれば良いのでたとえば'a123b'でもOKになってしまいますがおそらく意図した動作ではないでしょう。

パラメータは1でなければならないのか、数値でないといけないのか、数値だとしたらどの範囲を受け入れるのかが判然としませんので、過不足があるかどうかはわかりませんが、その点を明確にすれば自ずと何を判定しなければならないかも自明になるでしょう。

正規表現でのチェックは案外ミスをしやすいので、たとえば数字の羅列であることを判定しないならctype_digitを使う(マイナス記号が含められませんが)、その上で範囲の条件もあるならintにキャストして数値比較する、など出来るだけ単純な方法を選んだ方が良いです。

正規表現を使う場合は、`/\Apattern\z/s'と、\A\z(それぞれ文字列先頭と末尾)で囲むことシングルラインモードの指定をすることを忘れないようにしてください。

パラメータの取り扱い方や検証方法など含めて、Webアプリケーションで注意すべきことが一通り載っている本として「体系的に学ぶ 安全なWebアプリケーションの作り方」(ISBN 4797361190)をお勧めしておきます。

Comments

[yukihiro_]

解りやすい回答をいただきありがとうございました。

正規表現は確かにミスがありました、ご指摘いただきありがとうございます。
その他の部分も大変参考になりました。

Answer 2

category=1 の元になっているデータをアプリが持っているのではないでしょうか？

DBに「カテゴリーテーブル」があったり、あるいは、PHP のコードで次のような配列が定義されていたり。

$categorys = array(
    1 => "あああ",
    2 => "いいい",
    3 => "ううう",
    4 => "えええ",
);

であれば、そのデータを用いてチェックすると良いと思います。例えば↑のような配列があるのなら、

if (isset($categorys[$category_no]) === false) {
    // なにかおかしい
}

のようにです。

category が「任意の整数」なのであれば別ですが、category という名前から察するに、既知のリストからの選択になっているはずだと思います。

---

もし、DBに「カテゴリーテーブル」があって、運営者などの操作でその一覧が増減することがあるのであれば（つまり「カテゴリーテーブル」が可変）、利用者がページ上でカテゴリを選択してからそのページをサブミットするまでの間に、カテゴリの数が減ってしまうことを想定して、上記の「なにかおかしい」の部分で表示するメッセージはマイルドなものにしておいたほうが良いかもしれません。

例えば「不正なリクエストです」などとメッセージを表示してしまうと、通常の利用者に（たまたまタイミングが悪かったため）このメッセージが表示されたときに「犯罪者呼ばわりされている」という印象を与えることがあるかもしれません。

Comments

[yukihiro_]

解りやすい回答をいただきありがとうございました。

確かに条件に漏れた場合の対処方はおっしゃる通りですね。
条件外の場合の対処を変えようと思います。

Answer 3

カテゴリーが必ず整数なら is_int() でのチェックも入れてみてはいかがですか？もしチェックする場合はGETはstring型の値になるのでif文の前にintにキャストする必要があります。それであればpreg_matchも外せます。それと!emptyは0もfalseになりますけど大丈夫ですか？もし0もあるなら以下の2つのうちどちらかを使うといいと思います。
if ($_GET["category"] != "")
if (strlen($_GET["category"]) > 0)

それからチェックの後も大切です。
HTMLに出力なら htmlspecialchars
MySQLからデータを取得するならプレースフォルダーを必ず使ってくださいね。

Comments

[suzukis]

intにキャストした後であればis_int()は必ずtrueになります。

[yukihiro_]

回答をいただきありがとうございました。
皆さんわかりやすい解説で助かりました。