質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

89.18%

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

解決済

回答 3

投稿

  • 評価
  • クリップ 0
  • VIEW 412

creative_09

score 52

書籍に掲載されていたコードですが、
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インジェクションに対しても有効なのであればこういった書き方をしたいとおもっています。
?に対して順番にセットしていくだけでも良いのでしょうか?

よろしくおねがいします

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

質問への追記・修正、ベストアンサー選択の依頼

  • creative_09

    2019/12/22 15:44

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

    キャンセル

  • asahina1979

    2019/12/22 16:19

    SQL

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

    キャンセル

  • creative_09

    2019/12/22 17:38

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

    キャンセル

回答 3

checkベストアンサー

+3

おす!

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クエリのパーツにしなければ心配する必要はねぇだろうな。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/12/22 15:38

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

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

    キャンセル

  • 2019/12/22 18:22

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

    キャンセル

  • 2019/12/22 19:21

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

    キャンセル

+2

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

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

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

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/12/22 15:40

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

    キャンセル

0

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

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

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

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

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/12/23 17:12

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

    キャンセル

  • 2019/12/23 19:42

    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

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

    キャンセル

  • 2019/12/24 15:17

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

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 89.18%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる