CODEWITHSUNDEEP
phpsqldatabasepdocode-injection
Uğur Gümüşhan
Uğur Gümüşhan

Reputation: 2543

Is building queries with binding parameters in PDO injection safe?

This is the query I am preparing. This looks too much like a normal mysql_query and I am unsure if I am out of the safe boundaries of PDO. 

function opinionlist($orderby="dateposted desc",$page="0",$pagesize="10"){
  $dbh = new PDO(...);
  $s = $dbh->prepare("select * from fe_opinion 
                       order by :orderby limit :page,:pagesize");
  $s->bindParam(":orderby", $orderby);
  $s->bindParam(":page", $page);
  $s->bindParam(":pagesize, $pagesize");
  $s->execute();
  $opinionlist = $s->fetchAll(PDO::FETCH_ASSOC);
  echo json_encode($opinionlist);
}
  1. Can I safely build queries like this?
  2. Is getting a table name for an order by statement safe or should I ONLY get values from input?

currently I changed my code to

function opinionlist($orderby="dateposted desc",$page="0",$pagesize="10"){
  $orderbylist=array("dateposted desc","countcomment desc","countvote desc");
  $dbh = new PDO(...);
  if(!in_array($orderby, $orderbylist)){$orderby="dateposted desc";}
  $s = $dbh->prepare("select * from fe_opinion order by $orderby limit :page,:pagesize");
  $s->bindParam(":page", $page);
  $s->bindParam(":pagesize, $pagesize");
  $s->execute();
  $opinionlist = $s->fetchAll(PDO::FETCH_ASSOC);
  echo json_encode($opinionlist);
}

Upvotes: 0

Views: 773

Answers (1)

Colin Pickard
Colin Pickard

Reputation: 46663

It's good practice to validate the type and contents of your inputs.

You can sanitize $orderby with mysql_real_escape_string() or reject values for $orderby which don't consist of a single valid column name followed by an optional asc or desc (or even a comma separated list of these, if you wish). You could determine your list of valid columns either by hard coding or requesting a list of columns for that table (from INFORMATION_SCHEMA).

And of course you can use is_numeric() for $page and $pagesize.

With that in place, not only are you covering against sql injection, but also making your code more robust.

Small update: as you discovered, you can't use a parameter for $orderby.

Upvotes: 1

Related Questions