SQLインジェクションの危険性について[MYSQL]

質問

下記のコードはPHPプログラマ一年生の自分から見ても、
POST入力値をチェックしていない危険なコード(一部を抜粋)だと
思うのですが先輩は問題ないと言い、修正する気がありません。
そこで、SQLインジェクションのようなSQL文を送信された場合に
ユーザー名とパスワードが表示されれば納得すると思い
いろいろテストしてみたのですが、うまく表示されません。
実はSQLインジェクションなどの問題ないソースなのでしょうか？
ご教授、よろしくお願いいたします。

■テーブル構成（ユーザ・パスワード管理テーブル）
CREATE TABLE `sample`.`test` (
`user_id` INT NOT NULL ,
`passwd` VARCHAR( 32 ) NOT NULL
)

■問題の処理分
<?php

$user_val = "" ;
$pass_val = "" ;

if (isset($_POST["user_id"]) == TRUE)
{
　　$mysql_c = mysql_pconnect('******' , "******" , "******");
　　mysql_select_db("******" , $mysql_c);
　　$sql = 'select * from test where user_id = ' . $_POST["user_id"] ;
　　$result = mysql_query($sql , $mysql_c);

　　if ( $result != false)
　　{
　　　　if(($row = mysql_fetch_object($result)) != false )
　　　　{
　　　　　　// パスワードチェック
　　　　　　if ( $row->passwd != $_POST["pass"] )
　　　　　　{

　　　　　　　　echo "check NG" ;
　　　　　　}
　　　　　　else
　　　　　　{
　　　　　　　　$user_val = $row->user_id ;
　　　　　　　　$pass_val = $row->passwd ;
　　　　　　}
　　　　}
　　}
}

?>

<FORM action="index.php" method="post" >
<input type="text" name="user_id" value="<?php echo $user_val ?>" >
<input type="text" name="pass" value="<?php echo $pass_val ?>" >
<input type="submit">
</FORM>

回答No.3

masa6272 · ベストアンサー

結論から言えば、この場合はSQLインジェクションの問題はありません。

mysql_queryは、複数クエリーの実行を許してないからです。
ユーザーがどんな値を入れても、失敗するだけで、害はありません。
1 OR 1=1
なんて値を入れると、testテーブルの全データが取得されますが、別にそれを無条件に表示しているわけではないので。
結果として最初に持ってきた行で、偶然パスワードが一致したら表示されるかもしれませんけれど・・・
それは、でたらめにパスワードを入れてログインできる確率と一緒です。

PostgreSQLだと、話が変わります。
複数クエリーを一回のpg_queryで実行できます。
user_id　に

1;DROP TABLE TEST

などと、入れられたらやばいです。

この場合は危険性はないとしても、習慣としてインジェクション対策を入れておくことはいいことです。
インジェクションの可能性があるかどうかを考えずに、常に対策をしておく。これで、し忘れがなくなります。

SQLインジェクションを防ぐ一番手っ取り早い方法は、prepareを使うことですが、mysql関数群は、これがないんですね。
mysqliを使う、PEAR::DBを使う。PHP5でしたら、PDOを使うことをお勧めします。
どうしてもmysqlを使うのでしたら、mysql_real_escape_stringを使ってください。

お礼

ご回答ありがとうございました。
SQLインジェクションの問題は無いとわかりましたので安心致しました。
自分も「習慣としてインジェクション対策を入れておく」には賛成です。
コードの量が増えるとバク混入率がUPすると言われますが、
ソースは常に最善の状態にすべきだと自分は考えています。
どこまでやるか のバランスが難しいところなのかも知れませんが・・・
これからも徐々に勉強したいと思います。
たいへん参考になる回答ありがとうございました。

回答No.2

yambejp

sql文のエラーを防ぐために、データは最低限でもクォーテーション
でくくっておいた方がよいでしょう。

あとは、
(1)当該ページが誰を対象にしているか
(2)どのようなデータを取り扱っているか
によって対策方法も自然と変わってくると思います。

(1)に関しては、不特定多数を対象としているのであれば、
送られてくるすべてのデータは信頼できない前提でインジェクション
対策が必要ですが、限られたユーザーのみの利用であれば、
費用対効果もあるのであまり厳密にすることもないでしょう。
管理者しか使わないようなものであれば、ある意味、
非常に適当でもかまいません。

(2)に関しては、個人情報などが含まれる場合はコンプライアンス
もありますので、不要なユーザーがアクセスできないようにする
かなり厳重な処理が必要でしょう。

お礼

今回の質問したコードは管理者が利用する処理です。
費用対効果という面からみると現状のままで問題ないことが
わかりました。
また、ソースを修正する場合に時間・費用・効果なども考え、
総合的に判断する力も必要と痛感されました。
とても参考になる回答ありがとうございました。

回答No.1

denbee

可能性があるとすると、user_idの部分に必ずSQL文が成功し、かつ全てのレコードが対象になる条件文を付け加えることだと思います。

ただ、DBのuser_id部分の定義はintですから、画面の入力の段階で
入力文字チェックを行っている可能性があります。
であれば、上記のような条件文の入力はできませんから、
SQLインジェクションも不可能となります。

お礼

ご回答ありがとうございます。
クライアント側で入力値を
Javascriptでチェックしていました。
ただし、サーバー(PHP)側でも数値チェックを
行った方が良いと自分は感じました。
とても参考になる回答ありがとうございました。