HelloWorldプラスアルファからさらに上を目指すために (PHP編)
以下、 ギークなお姉さんは好きですか <title>をクールにしてみた! より。
extract($_GET);
mysql_connect('localhost','ユーザ名','パスワード');
mysql_select_db('データベース名');
mysql_query("set names utf8");
$sql="select * from geekDB where name_id = "$name_id"";
$result = mysql_query($sql);
while ($item = mysql_fetch_array($result)) {
echo "<title>";
echo $item["name"];
echo "</title>";
}
すでに誰かが突っ込んでいるが、extract($_GET); はまずい。 でもなぜまずいのかどこにもマニュアルにも書いてないのがなんともねえ。
// このコードがfoo.php だとする。 $himitu = "ok!" extract($_GET); echo $himitu;
http://example.jp/foo.php?id=hoge へのアクセスの結果↓
ok!http://example.jp/foo.php?id=hoge&himitu=baka へのアクセスの結果↓
bakaあれれれれ?誰かに書き換えられる想定にはない固定値のはずの$himitu変数の中身が書きかえられちゃったよ!? ということになる。 $himituという変数の存在に気づかれる可能性は低いから大丈夫じゃん?とか言っている人はごく近いうちにほかの形での脆弱なコードを量産する人になってしまうだろう。 mixiでサーバ障害だかうっかりミスだかでソースコードが見えちゃった事件っていう実例もある。
mysql_query("set names utf8"); もまずい。→
SET NAMESは禁止。
mysql_set_charset
っていうのがあるんだからそっち使おう。つまり
mysql_set_charset("utf8");
ただしこの関数はPHP5.2.3以上じゃないと使えないというのがネックだ。
次。
$sql="select * from geekDB where name_id = "$name_id"";
そもそもこれだとダブルコーテーションの配置がおかしいからsyntax errorなはずなんだけど、コピペミスだろうか。
それはともかく、「SQL文の組み立てで$_GET["name_id"]って書くのは面倒だからextractして$name_idでアクセスできるようにしてしまえ」と思ったのかもしれない。でもそこは我慢しよう。
$sql = "select * from geekDB where name_id = '" . $_GET["name_id"] ."'" ;
あるいは
$sql = "select * from geekDB where name_id = '${_GET["name_id"]}'" ;
ところで、${hogehoge}って書くことでどこまでが変数名なのかを明示できることを知らない人は意外と多いみたい。
もちろんこれでもまだまずい。
元記事のコメントでは、phpのmysql関数群のでは 「;(セミコロン)で区切るような複合クエリに対応してないから大丈夫だろう」的なことが書いてあるが、SQLインジェクション攻撃はなにもセミコロンで区切る方法だけじゃない。
さぼらずちゃんとmysql_real_escape_string() を使おう。
$sql = "select * from geekDB where name_id = '"
. mysql_real_escape_string($_GET["name_id"]) ."'";
そもそもの話になるが、PHPには実はmysqlにアクセスするためのクラス/関数群が三つもある。
- MySQL関数。元祖。古い。冒頭のコードで多用されていたやつ。
- MySQL 改良版拡張モジュール。少し新しいやつ。
- PDO - PHP Data Objects 一番新しいやつ。mysqlに限らない汎用モジュール。
PHP: プリペアドステートメントおよびストアドプロシージャ - Manual
プリペアドステートメントに渡すパラメータは、引用符で括る必要は ありません。それはドライバが自動的に行います。 アプリケーションで明示的にプリペアドステートメントを使用するように すれば、SQL インジェクションは決して発生しません (しかし、もし信頼できない入力をもとにクエリの他の部分を構築している のならば、その部分に対するリスクを負うことになります)。
セキュアになるだけじゃなく、手作業でSQL文組み立てるよりもコードの見た目がわかりやすくなるという利点もある。
<?php
$dbhandle = new PDO('mysql:host=localhost;dbname=データベース名', $user, $pass);
$dbhandle->query("set names utf8;"); // ←さっき書いたとおりでほんとはやりたくないんだが
// ほかに方法がないかも。本来ならDBサーバのmy.confファイル上かなにかで
// 設定すべきなのだろうが詳しくは知らん。いや、php.iniファイルかな?
// select * じゃなくちゃんとカラム名まで指定する。ここは好みかもしれないけど。↓
$statement = $dbhandle->prepare("select geekname from geekdb where name_id = ?");
$statement->bindParam(1, $_GET["name_id"]); // 上の?マークにあてがう
$statement->execute(); // SQLの実行
$row = $statement->fetch(); // 1件しか引っかからないとわかっているなら
// ループさせるまでもなく1行目だけとれば(「フェッチ」すれば)いい。
// 引っかからなかった場合(ゼロ件ヒット)のエラー処理が抜けているのはまずいんだけど
// ここではとりあえずの模式。
$name = $row[0]; // これでひっかかったDBレコードの1行目の1カラム目が$nameに入る。
$dbhandle = null; // 念のため接続閉鎖
// あとは、phpな部分↑とHTMLな部分↓は極力分けよう。
?>
<html>
<head>
<title><?php echo $name;?></title>
<body>
........
なお、プリペアドステートメントじゃ表現できないようなSQLを作らなきゃならない局面も必ずある。 そういうときは、pdo::quoteを使う。
まとめ:
- ある程度まで来たら、巷の初心者向けの本は捨てて、 PHPのマニュアルを読むようにしよう。PHPのオンラインマニュアルはとても充実していて、巷の本のほうが実はそれをパクっていることすらよくある。
- セキュアなコードを書くのは、めんどくさい。 「セキュリティ大丈夫ですか?」ってツッコまれたところで「もっと具体的に言えよセキュリティって言いたいだけちゃうんかハゲ」と思うのは俺だけじゃないだろう。 そういう意味でも、セキュリティ過敏症というやつは大嫌いだ。自分の案件の納期が迫ってるからという話でもないのにフレームワーク使えの一言で済ませることしか考えてない人もね。
- でも、ある程度は慣れの問題だから、ちょっとだけ我慢しよう。 それから、ほっといてもセキュアになるようなアリモノを優先的に使うことも有効だ。 プリペアドステートメントもそんなアリモノの一種だと考えてもいいだろう。 フレームワークみたいな大きなアリモノをいきなり使い始めるよりも先に、こういうレベルでの小さなアリモノを使うことからその考え方を学ぶのも大切なんじゃないだろうか。
- あとは、モリモリ書こう。そのうち、関数やクラスで共通化する、includeでファイルを分ける、例外処理をちゃんと使う、テンプレートエンジンを使う、フレームワークを使う、MVCが、、、ってことになってゆく。
ここから追記:
see also:

コメントする
(初めてのコメントの時は、コメントが表示されるためにこのブログのオーナーの承認が必要になることがあります。承認されるまでコメントは表示されませんのでしばらくお待ちください)