POST・GETで受けた値をデータベースでSELECTする際のセキュリティ

Question

1.商品IDをGETで受け取る
2.データベースから受け取ったIDで商品を検索する

この場合において、

$product_id = h(mb_convert_kana($_GET['product_id']));

$sql = "SELECT * FROM product WHERE product_id = :product_id";
・・・・・・・・・・・

のような形だったとします
この場合、
不正な値が入力される場合を想定してGETの値をバリデーションしたほうがいいのでしょうか？
それとも、該当するproductはデータベースにはないわけだから、
データベースへの問い合わせをtry/catchしてエラーを返すだけでも問題ないのでしょうか？

バリデーションするとなれば、
値が
空ではない
数字
6桁以内
など、常にしないといけなくなるのかな？と思い質問させていただきました

よろしくおねがいします

Answer 1

マニュアルの一歩先を記事としてまとめてくれている人がいるので参考まで。

PHPでデータベースに接続するときのまとめ
$_GET, $_POSTなどを受け取る際の処理

あとこの辺も読むと良いです。
バリデーションには3種類のバリデーションがある 〜 セキュアなアプリケーションの構造 〜

Comments

[creative_09]

ありがとうございます
すべて目をとおしておきます。

[退会済みユーザー]

読んだら、BA につけたコメント、アップデートしておいてください。
間違った解釈しているので。

以下。雑に指摘しておきます。
・プログラミングの正しい動作に入力バリデーションは必須
・プリペアドステートメントは特定の環境では、完全なセキュリティ対策にはならない

[creative_09]

ありがとうございます。

Answer 2

問題を2つに切り分けて考えた方が良いと思います。

1. SQLインジェクション対策

書かれている、$sql 変数の :product_id に $product_id を割り当てる方法が不明なため、(この後の行で書かれていることと思います)、厳密な答えはできませんが、PDO のプリペアドステートメントを使われているようであれば問題ないと思います。

おそらく

$stmt = $dbh->prepare($sql);
$stmt->bindParam(':product_id', $product_id);

こうなっているでしょうか。これであれば、SQLインジェクションへの対策は十分であり、これ以外の方法を使ってはいけません。

1. 検索結果が0、2つ以上だった時のエラーハンドリング

単純な try/catch でも良いのですが、検索該当行数を見て処理を分岐したほうが、本当の例外(DBがダウンしているとか) の判断がしやすいので、結果0だった場合と結果2以上だった場合で分岐されることをおすすめします。

それ以前に
おそらくWebフレームワークを使われているようですが、その場合は生SQLは書かずに、Webフレームワークで準備されているORマッパーを使った方がメンテナンス性・安全性の面からも適しています。
もしCakePHPをお使いであれば
https://book.cakephp.org/3/ja/orm.html こちらをご覧ください。

Comments

[creative_09]

回答ありがとうございます
SQLインジェクションにつきましては提示して頂いたとおりです。
詳しく書いていなかったのですみません。。。

フレームワークは使っておりません。触ったこともありません。
PHPの学習後にlaravelを覚えたいと思っています

今回のケースでは該当が2件あるケースではなく、オートインクリメントに設定された数値ですが、
もし、2件あった場合、こちらでどっちのパターンでエラーがでたのかがわかるようにするということでしょうか？

また、確認となってしまいますが、
プリペアドステートメントをしていて、今回のケースの場合は、とくにバリデーションをしていなくても、結果が該当なしになるだけで、負荷がおおきくなるなど、その他の問題につながる可能性は少ないと考えてもいいのでしょうか？

[ytyng]

> フレームワークは使っておりません
h 、という関数をつかわれていたので、何かのフレームワークをつかわれていたのかと思いました。

> オートインクリメント
であるならば、2件の検索結果は考慮しなくてもよさそうですね、。

> 負荷がおおきくなるなど
無いと言って良いと思います。
もし、不都合のある入力方法を見つけられたら、ぜひ開発者への連絡をしていただきたく思います。

ただし、= 以外の絞り込み条件、例えば LIKE 句等を使われている場合は、検索語句によって負荷が変わわってきます。( '%' での検索など)

[creative_09]

なるほど。ありがとうございます。
今回の場合は、オートインクリメントの値でLIKE句など、条件などもなしですが、
たとえ、GETで受け取ったパラメーターだったとしても、バリデーションをし、
不正な値がきても適切に前段階で処理していけるようにします。
面倒ですがそれが基準と考えます。なにかってからでは遅いので。。。
ありがとうございました。

Answer 3

外部からくるパラメータはいくらでも細工を仕込めるので、
不適切なデータが紛れ込まないか常にチェックするよう心がけたいと考えます。
（SQLインジェクションだけじゃないでしょ。）

ところで、h関数がネットでよく見る事例だとしたら、
データベースでの検索用に使う場合はhtmlspecialchars()にかける前のピュア(?)なまま
使うことをおすすめします。

Comments

[creative_09]

回答ありがとうございます。
パラメーターは外部からいくらでも細工される
なので、今回のような場合でも、受取る値を完全にバリデーションしてからデータベースに問い合わせたほうが良いということでしょうか？
もし、それが単純にプリペアドステートメントをしていれば「結果が該当なし」くらいの結果でエラーをだせばいいのかなぁ？と思った次第です。

SQL文に組み込む変数は生のままのhtmlspecialchars()を使わないほうがいいということで、
補足アドバイスありがとうございます