0
function delete_group($db) {
    $ids = Parameters::get('ids');
    $ids = implode(',', $ids); // now a string like '5,6,7'.
    add_to_log($ids);
    try {
        $stmt = $db->prepare("DELETE FROM mytable WHERE id IN (:ids)");
        $stmt->bindParam(':ids', $ids, PDO::PARAM_STR);
        $stmt->execute();
        response('success', 'success', NULL);
    }
    catch (PDOException $e) {
        response('error', 'Delete group failed.', NULL);
    }
}

This code doesn't work: only the first row is deleted. But if I do

$stmt = $db->prepare("DELETE FROM mytable WHERE id IN ($ids)");

instead (just insert the string), it works, though the code has the SQL injection security issue. How to make it work and keep secured?

j08691
  • 204,283
  • 31
  • 260
  • 272
noober
  • 4,819
  • 12
  • 49
  • 85
  • You can't do that. You need to bind them individually; SQL bound parameters are not arbitrary variable interpolations/concatenations but rather individual values passed in places where accepted. – Michael Berkowski Jan 05 '14 at 22:21
  • If I remember correctly PHP PDO doesn't support `WHERE IN` for prepared statements. You'll have to do some extra work to get it set up (either create X `?` params depending on the size of the array or do an `implode()` on the array). – Sam Jan 05 '14 at 22:22

1 Answers1

3
$ids = Parameters::get('ids');
$ids = array_map('intval', $ids);
$ids = implode(',', $ids);

Now you don't have to worry about injection.

Jessica
  • 7,075
  • 28
  • 39
  • This is the best answer from all ones to both questions linked above. – noober Jan 05 '14 at 22:32
  • @noober It may be a short solution, but it is only valid for integer ids. For string values it can be done as well, but it's not recommended to mix bound params and escaped strings in a larger query. Bound params in prepared statements are the accepted best practice even if the line count increases significantly. – Michael Berkowski Jan 05 '14 at 22:37
  • @MichaelBerkowski Never saw non-integral ids. – noober Jan 05 '14 at 22:42
  • @noober That doesn't mean they don't exist, or that an `IN ()` clause is only ever used on id PK columns. – Michael Berkowski Jan 05 '14 at 22:43
  • Since the comment was "// now a string like '5,6,7'." I'm going to assume they are integers. – Jessica Jan 05 '14 at 23:21