0

Given the following code

 <?php
$values = array('Foo'  =>'Foo'  ,'Bar'  =>'Bar'  );
$separated = "'" . implode("','", $values)."'";
$sql = 'SELECT NAME,AGE FROM CATS WHERE TITLE IN('  .$separated.')'  ;

print_r($sql);

produces:

 SELECT NAME,AGE FROM CATS WHERE TITLE IN('Foo','Bar')

Is there anything I need to be aware of about SQL injection using this type of query builder? If so, what is an attack that can occur?

Woot4Moo
  • 23,987
  • 16
  • 94
  • 151

2 Answers2

2

The only rule of SQL security:

NO value should be added to a query directly, but via placeholder only

So, you have to use a library that supports placeholders.

Assuming your database is mysql, the best choice would be safemysql, which will let you have as simple code as this:

$sql  = 'SELECT NAME,AGE FROM CATS WHERE TITLE IN(?a)';
$data = $db->getArr($sql, $values);
print_r($data);

or you can use PDO, but it will take you a lot more trouble

Community
  • 1
  • 1
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • 1
    I use PDO and have a setup like this: `$result = $this->db->query('SELECT FROM users WHERE id IN (' . $this->getPlaceholders($userIds) . ')', $userIds);` Where the function just returns `implode(',', array_fill(0, count($array), '?'))` or `null` – JimL Sep 04 '13 at 12:57
  • See, it is much worse than safeMysql's custom placeholders, isn't it? ;) But for the PDO world your solution is nearly brilliant though – Your Common Sense Sep 04 '13 at 12:58
  • Yep, easier is (almost) always better :) Just wanted to share it in case OP (or others) were bound to PDO as I am and wanted a solution. Oh, and any improvement suggestions are appreciated. – JimL Sep 04 '13 at 13:01
  • You could make it an answer, and it will be a good one. Think of the future readers. A nicely formatted answer contains all the code and example is better than sketch in the comments. – Your Common Sense Sep 04 '13 at 13:05
  • Not sure if it's relevant as an answer, OP never said he was using PDO :) – JimL Sep 04 '13 at 13:07
  • @JimL feel free to make it an answer, future readers dont tend to focus on comments too much and i am going to try the PDO version as well. – Woot4Moo Sep 04 '13 at 13:08
2

You should never use any variables in queries no matter where they come from. A solution for PDO and parameterized queries will be to add placeholders to the query.

I do it something like this:

function getPlaceholders ($array) {
  return !empty($array)
    ? implode(',', array_fill(0, count($array), '?'))
    : null;
}

$userIds = array(1,2,3,4);

$sql = 'SELECT FROM users WHERE id IN (' . $this->getPlaceholders($userIds) . ')';
$result = pdo_query($sql, $userIds);

Normally you would have this in a OOP-format.

$userIds = array(1,2,3,4);

$sql = 'SELECT FROM users WHERE id IN (' . $this->getPlaceholders($userIds) . ')';
$result = $this->db->query($sql, $userIds);


// common file which is extended
public function getPlaceholders ($array) {
  return !empty($array)
    ? implode(',', array_fill(0, count($array), '?'))
    : null;
}

This will generate a query like:

SELECT FROM users WHERE id IN (?,?,?,?)
JimL
  • 2,501
  • 1
  • 19
  • 19
  • Just a note. Sadly, but for `NOT IN (NULL)` it will return no records as well as `IN (NULL)`, while I would expect rather opposite behavior. It bugs me a little but no solution has been found yet – Your Common Sense Sep 04 '13 at 13:34