データベースへの登録、更新、削除について SQLインジェクション

Question

書籍に掲載されていたコードですが、
SQLインジェクション対策ができているのでしょうか？

function executeSQL($sql, $array){
    try{
      if(!$pdo = $this->Connectdb())return false;
      $stmt = $pdo->prepare($sql);
      $stmt->execute($array);  
      return $stmt;
    }catch(Exception $e){
      return false;
    }
  }

  function InsertGoods(){
    $sql = "INSERT INTO goods VALUES(?,?,?)";
    $array = array($_POST['GoodsID'],$_POST['GoodsName'],$_POST['Price']);
    parent::executeSQL($sql, $array);
  }
  
  function UpdateGoods(){
    $sql = "UPDATE Goods SET GoodsName=?, Price=? WHERE GoodsID=?";
    //array関数の引数の順番に注意する
    $array = array($_POST['GoodsName'],$_POST['Price'],$_POST['GoodsID']);
    parent::executeSQL($sql, $array);
  }

とあったのですが、
INSERTやUPDATEする際には
プレースホルダを使い、SQLインジェクション対策をする
と思っています。
また別のコードとなりますが自分の書いたコードでは

    public function create($user_id, $zip1, $zip2, $pref_code, $city, $address, $name01, $name02, $tel)
    {
        $sql = "INSERT INTO address (
            user_id, zip1, zip2, pref_code, city, address, name01, name02, tel, create_date, update_date)
            VALUES (:user_id, :zip1, :zip2, :pref_code, :city, :address, :name01, :name02, :tel, now(), now())";
        $stmt = $this->pdo->prepare($sql);
        $stmt->bindValue(':user_id', $user_id);
        $stmt->bindValue(':zip1', $zip1);
        $stmt->bindValue(':zip2', $zip2);
        $stmt->bindValue(':pref_code', $pref_code);
        $stmt->bindValue(':city', $city);
        $stmt->bindValue(':address', $address);
        $stmt->bindValue(':name01', $name01);
        $stmt->bindValue(':name02', $name02);
        $stmt->bindValue(':tel', $tel);
        $stmt->execute();
        $_SESSION['message_act'] = "登録しました。";
        header('location: address.php');
    }

としています。
正直、書籍のものがコードが少なく、SQLインジェクションに対しても有効なのであればこういった書き方をしたいとおもっています。
?に対して順番にセットしていくだけでも良いのでしょうか？

よろしくおねがいします

Comments

[m.ts10806]

何でしょうその書籍って。
また代表的なSQLインジェクションの攻撃方法は試してみましたか？

[creative_09]

書籍はMYSQLのツボが絶対にわかる
という本です
この時点では、SQLインジェクションの対策がほどかされておらず、
ほかの部分でエスケープされている可能性もあるのでしょうか？
今回のコードのみでは対策はされていない。というのが答えなのかもしれません。
それもおっしゃるとおり、SQLインジェクションの攻撃を試してみるのが一番はやいのかもしれません。
そういう方法を知らないので調べた後、自己解決できるようにしたいと思います。

[m.ts10806]

その書籍持ってないのでよく分かりませんが、IPAのサイトは確認しておいてください。
きちんと理解を深めたいのでしたら通称 徳丸本を買うのも手です

[退会済みユーザー]

とりあえず、 Linux では正常に動かないなこれ

[creative_09]

クラスの定義部分から書いておらず、publicなども記載しませんでした。
そのためでしょうか？
ほかの原因でしょうか？

[退会済みユーザー]

SQL

LINUX上の場合、初期設定だとテーブル名の大小は別々

[creative_09]

そうなんですね
覚えておきます
ありがとうございます。

Answer 1

おす！

SQLインジェクション対策が出来ているかいねぇかって話なら、出来てるぞ。

どこを見ればいいのかって話だが、prepareしてるSQLがどうなってっか見ればいいぞ。

prepareしてるSQLには一切変数展開がされてねぇから、書いたとおりにしか動かねぇ。

INSERT INTO goods VALUES(?,?,?)

と

UPDATE Goods SET GoodsName=?, Price=? WHERE GoodsID=?

だな。

SQLインジェクションってぇのは、SQLがデータベースのオプティマイザで解析されて実行計画が立てられる時に、実装者の意図とは違った構文として認識された場合に発生すんだ。

例えばGoodsNameの値が

'; ---

だったりして、これが直接埋め込まれた場合、

UPDATE Goods SET GoodsName='';　---', Price=? WHERE GoodsID=?

みてぇになる。

これをオプティマイザが解析してクエリの実行計画を立てると、WHERE文がコメントアウトされたせいでUPDATE文が意図せず無作為に実行されてデータベースが不正に書き換えられちまうって寸法だ。

だけんども、今回発行してるクエリは

UPDATE Goods SET GoodsName=?, Price=? WHERE GoodsID=?

だ。オプティマイザはこれを解析して実行計画を立てる。

その後で、 ? の場所に値をバインドする。

既にクエリの実行計画は立ててあっからおかしなUPDATE文が発行されちまう事は絶対にねぇ。
? の場所に値をバインドしてくだけだ。

SQLインジェクションってぇのは、構文も値も全部ひとかたまりの文字列としてデータベースに渡されて意図しない形で解析されっから起こる。

渡したSQLクエリが製作者の意図に反した解釈がされる可能性が全く無い状態でprepareしてさえいれば、SQLインジェクションは絶対に起こらねぇんだ。

だから、注意しなければならねぇのはprepareするSQLクエリを動的に組み立てる時だけだな。

その場合でも、ユーザーからの入力を直接SQLクエリのパーツにしなければ心配する必要はねぇだろうな。

Comments

[creative_09]

ありがとうございます！
非常に分かりやすいです
まず、答えとして出来てるということなので、
そこからの根拠を説明してもらえるのはすごくありがたいです。
なぜそうなのかはある程度ここでキーワードを得て調べるつもりでしたので。

でも理解は難しいですね。
言葉がむずかしいのかもしれません。
要は、sql分は?であろうが、パラメーターであろうが、一旦その形で命令を固定される。
そこの部分は変えられない。
なので、あとで入ってくる値に命令文は左右されない。
そういう仕組み。
ということで理解しました。
あってるのでしょうか

[退会済みユーザー]

そうだな、そういう考え方でいいと思うぞ。

[creative_09]

ありがとうございます。
また質問答えて頂ければうれしいです
よろしくお願いします

Answer 2

SQLインジェクション対策という意味では
どちらの方法も同様に有効です。

使い分けについては
PHPマニュアル

の説明とサンプルを読んでみるとイメージが掴めるかもしれません。

Comments

[creative_09]

ありがとうございます。
なるほど、とりあえずこれはインジェクションの対策はされているということですね。
参考マニュアルもありがとうございます。助かります

Answer 3

記述の範囲では、SQL インジェクション対策が適切になされているかどうか判断できません。

提示範囲外なので余談
SQL インジェクションはバインド機構を使用することでおおよそ防ぐことが可能ですが、PDO インスタンス生成時にパラメータを正しく渡してない場合、ごく限定的な条件下で、SQL インジェクションの危険性を残します。

対策自体はごく簡単で dsn のオプションとして DB に合致したcharsetを渡し、SET NAMESを使用せず、更に安全に倒すのであれば、コンストラクタに渡すオプションとしてPDO::ATTR_EMULATE_PREPARES => false,
を設定すると良いです。

PDO による DB 接続に関しては、この記事をひと通り読んでみると良いです。
PHPでデータベースに接続するときのまとめ

Comments

[creative_09]

ありがとうございます。
難しいですが、サイトにも目を通しておきます。

[退会済みユーザー]

PDO を使用した自前の接続は、フレームワークを使用するまでの「つなぎ技術」なので、先に安全に使用するためのテンプレートとして理解することをオススメします。

PHP で MySQL 接続時に必要な知識(最小限版)
https://qiita.com/te2ji/items/56c194b6cb9898d10f7f

もちろんフレームワークも使いこなそうと思えば、接続に対しての理解も十分に深めなくてはならないので、回答のリンクは何度か読み返すと良いです。

本件に関しては、以下と合わせて読むと理解が深まります。

ぼくがPDOを採用しなかったわけ(Shift_JISによるSQLインジェクション)
https://blog.tokumaru.org/2010/07/pdo-shiftjis-sqli.html

古い記事なので、現在とは仕様が少しずつ違い(デフォルト値も変わっています)、問題となる構成は当時以上に限られたものになりますが、問題点の理解には非常に役に立つ記事です。

PDO::ATTR_EMULATE_PREPARES => false
に関しては、こちらを参考に、回答のリンク記事を検証してみてください。

【PHP】PDOの静的プレースホルダと動的プレースホルダの違いを確認する
https://qiita.com/7968/items/7ddd59b94eb5a4fb6eaf

文字コードの取り扱いに間違いがあると、なぜ危険かが理解できると思います。

[creative_09]

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