PHP string replace to prepare PDO SQL

Question

I have some SQL queries that use PHP string variables to create the query before PDO prepare().

$connection = new PDO(...);

// Make variable placeholder for each column.
$params = array();
foreach ($row as $col => $value) {
  $params[':' . $col] = $value;
}
$columns = implode(', ', array_keys($row));
$values = implode(', ', array_keys($params));

$query = "
  INSERT INTO my_table ($columns)
  VALUES ($values)
";

$statement = $connection->prepare($query);
$statement->execute($params);

Or something similar with SELECT:

$query = "
  SELECT field
  FROM my_table
  WHERE id IN ($ids)
";

Where the query will become

$query = "
  SELECT field
  FROM my_table
  WHERE id IN (:id0, :id1, :id2)
";

and then the execute() function will pass in the params like array(':id0' => 0, ...).

Is this vulnerable to injection if the part being inserted is just a bunch of placeholders to be used for query preparation? And is there a better way to do this in PHP with PDO?

Possible duplicate of [Can I bind an array to an IN() condition?](https://stackoverflow.com/questions/920353/can-i-bind-an-array-to-an-in-condition) — Dharman, Jul 16 '19 at 19:57
Short answer: Yes, it is still vulnerable to SQL injection, but it depends on where the parameter names come from. If they are constant values hardcoded in your code then you are safe, but if they come from any other source then you can never know what the value is. — Dharman, Jul 16 '19 at 19:59
If you are going to dynamically define the columns, they should be whitelisted first - to ensure there are no errors or no exploits. — Qirel, Jul 16 '19 at 20:01
When you have a dynamic number of parameters, it's usually easier to use `?` placeholders instead of named placeholders. — Barmar, Jul 16 '19 at 20:10
The columns are dynamic with regards to what values are populated and what are left empty, I think, but the column names themselves are hardcoded in code and not taken from user input. — mbomb007, Jul 16 '19 at 20:26

score 1 · Accepted Answer · answered Jul 16 '19 at 20:14

1

When binding a dynamic number of parameters, I revert to the ? placeholders. You can do:

$placeholders = implode(',', array_fill(0, count($values), '?'));
$query = "SELECT field FROM my_table WHERE id IN ($placeholders)";
$stmt = $pdo->prepare($query);
$stmt->execute($values);

You still need to do string substitution if the column names are dynamic, as in your INSERT example. Those should be white-listed to prevent injection. But you can use the above mechanism for the values being inserted. You'll need to use

$values = array_values($params);

because ? placeholders can't be filled in from an associative array.

answered Jul 16 '19 at 20:14

Barmar

741,623
53
500
612

I prefer the idea of associative parameters, because then the order does not matter when executing the query. For example, if your query had a parameter added before the existing IN statement, and a mistake was made where you forgot to pass it in, the query would be executed with parameters in the wrong places. – mbomb007 Jul 16 '19 at 21:40
That's true, there are pros and cons. – Barmar Jul 16 '19 at 21:41

PHP string replace to prepare PDO SQL

1 Answers1