SQLインジェクション対策

SQLインジェクション対策として、変数を直接SQLに埋め込むのではなく、PDOのprepareメソッドを利用して作成したステートメントに値をバインドする形式にする場合、ユーザー定義関数をどのように
修正すればいいですか？


function update_item_stock($db, $item_id, $stock){
$sql = "
UPDATE
items
SET
stock = {$stock}
WHERE
item_id = {$item_id}
LIMIT 1";
return execute_query($db, $sql);
}

function execute_query($db, $sql, $params = array()){
try{
$stmt = $db->prepare($sql);
return $stmt->execute($params);
}catch(PDOException $e){
set_error('更新に失敗しました。');
}
return false;
}

上記のユーザー定義関数を下記のようにbindValueを用いて表す場合、$stmt->bindValueの部分を
上記のどこに追加すればいいですか？

try {
$sql = 'UPDATE item_stock
SET stock = ?, update_date = ?
WHERE item_id = ?';
$stmt = $db->prepare($sql);
$stmt->bindValue(1, $update_stock, PDO::PARAM_INT);
$stmt->bindValue(2, $date, PDO::PARAM_STR);
$stmt->bindValue(3, $item_id, PDO::PARAM_INT);
$stmt->execute();
$complete_msg[] = '在庫変更に成功しました。';
} catch (PDOException $e) {
$err_msg[] = '更新に失敗しました。理由：' . $e->getMessage();
}

質問者からの補足コメント

• 

  回答ありがとうございます。 update_item_stock関数に$params = array('item_id' => $item_id, 'stock' => $stock);を追加して、？に変更しました。
  
  $stmt->bindValue(1, $params['stock'], 　　PDO::PARAM_INT);
  $stmt->bindValue(2, $params['item_id'], 　PDO::PARAM_INT);
  
  $params['stock']と$params['item_id']を一般化した形に直すことは可能でしょうか？

    補足日時：2021/04/22 18:12

• 

  execute_query関数を修正しない場合は、update_item_stock関数の{$stock} と {$item_id} の部分を？に変更するだけでOKですか？

  No.3の回答に寄せられた補足コメントです。 補足日時：2021/04/23 18:06

• もしexecute_query関数を修正せずにそのまま使う場合は、update_item_stock関数の{$stock} と {$item_id} の部分を？に変更するだけでOKですか？

  No.2の回答に寄せられた補足コメントです。 補足日時：2021/04/23 18:07

No.1 回答者： naoiz 回答日時： 2021/04/22 17:22

↓ここに追加すればいいんでないかと思います。

function execute_query($db, $sql, $params = array()){
try{
$stmt = $db->prepare($sql);

＜ここ＞

return $stmt->execute($params);
}catch(PDOException $e){
set_error('更新に失敗しました。');
}
return false;
}

{$stock}だとか {$item_id}は『?』にして、$paramでexecute_query()に渡してやる感じになると思いますけど。

No.2 回答者： Ogre7077 回答日時： 2021/04/22 18:22

取説の「例1 名前付けされたプレースホルダを用いてプリペアドステートメントを実行する」が参考になります

https://www.php.net/manual/ja/pdostatement.execu …

# SQL に名前付きプレースホルダを埋め込む
$sql = " ... WHERE stock = :stock ... ";

# パラメータ値を決める
execute_query($db, $sql, [ ':stock' => $stock ]);

# パラメータ値を結び付けて SQL を実行する
foreach ($params as $k => $v) {
_ $stmt->bindValue($k, $v, 明示的な型の指定);
}
$stmt->execute();

No.3 回答者： naktak 回答日時： 2021/04/22 22:00

プレースホルダーを名前付きにしないと、仮に同じ値を設定したい箇所が複数あったとき、その数分、同じ値を設定しなければなりませんので不便です。

Name,Value,Typeというプロパティを持ったクラス、もしくは連想配列を設けて、そこにパラメーターの情報を設定し、それを複数有する配列をexecute_queryに渡す。
prepareの後に、配列の個数分、パラメーターをバインドするとかすればいいのでは。

No.4 回答者： naoiz 回答日時： 2021/04/22 22:22

回答ありがとうございます。

update_item_stock関数に

$params = array();
$params[] = array('value' => $item_id, 'type' => PDO::PARAM_INT);
$params[] = array('value' => $stock, 'type' => PDO::PARAM_INT);

とかして、

foreach($params AS $key => $param){
$stmt->bindValue($key, $params[$key]['value'],$params[$key]['type']);
}

とかですかね。

No.5 回答者： naoiz 回答日時： 2021/04/22 22:24

NO.4です。

ごめんなさい、投稿ミスりました。

訂正
↓

$params = array();
$params[] = array('value' => $item_id, 'type' => PDO::PARAM_INT);
$params[] = array('value' => $stock, 'type' => PDO::PARAM_INT);

とかして、

foreach($params AS $key => $param){
$stmt->bindValue($key, $param['value'],$param['type']);
}

とかですかね。

No.6 回答者： Ogre7077 回答日時： 2021/04/24 15:43

No.2の回答の補足について

execute_query を変えず bindValue を使わない方式ならば
* {$stock} を :stock の名前付きプレースホルダに変更
* $params に [ ":stock" => 値 ] 形式の連想配列を渡す
の二点の変更となります。
疑問符のプレースホルダは間違いえやすいので避けるべきかと。

ただし、
PDOStatement::execute の取説に明記してある通り、
パラメータ値は全て PDO::PARAM_STR として扱われてしまい、
場合によっては不具合がおきるので、お勧めできません。
例) MySQL の LIMIT 句の値は整数型でないとエラー発生