How to replace mysql_escape_string by PDO::quote()

Question

I'm learning PHP using http://www.elated.com/articles/cms-in-an-afternoon-php-mysql/ that has been very useful so fare but cannot get my head around to replace the deprecated mysql_escape_string.

I've followed the existing conversation on StackOverFlow : https://stackoverflow.com/a/20294502/7157496 and I think that a possible solution will be to implement the quote() as $conn->quote($order) to avoid SQL injection but I do not see where it should feat in the code.

Or do you think that $st = $conn->prepare( $sql ); is already doing the job here?

  public static function getList( $numRows=1000000, $order="publicationDate DESC" ) {
$conn = new PDO( DB_DSN, DB_USERNAME, DB_PASSWORD );

/*$sql = "SELECT SQL_CALC_FOUND_ROWS *, UNIX_TIMESTAMP(publicationDate) AS publicationDate FROM articles
        ORDER BY " . mysql_escape_string($order) . " LIMIT :numRows";*/

$sql = "SELECT SQL_CALC_FOUND_ROWS *, UNIX_TIMESTAMP(publicationDate) AS publicationDate FROM articles
        ORDER BY " . $order . " LIMIT :numRows";

$st = $conn->prepare( $sql );
$st->bindValue( ":numRows", $numRows, PDO::PARAM_INT );
$st->execute();
$list = array();

while ( $row = $st->fetch() ) {
  $article = new Article( $row );
  $list[] = $article;
}

MinistryOfChaps · Accepted Answer · 2017-08-12T20:59:18.387

So your problem here is PDO only allows the binding of values and using PDO::Quote is not a safe alternative nor an efficient one.

Or do you think that $st = $conn->prepare( $sql ); is already doing the job here?

No it does not do the job, PDO::prepare only prepares binded values, not hard coded values.

Because your $order is taking input from a user (which can easily be manipulated) the safest option is to create an array of whitelisted order types that are permitted. If the input from $order is in the whitelisted array you can then proceed to prepare and execute the statement.

EDIT: Here is my alternative to your current code, taking into account the link in the comment:

<?php
public static function getList( $numRows=1000000, $order="publicationDate DESC" ) {

 $conn = new PDO(DB_DSN, DB_USERNAME, DB_PASSWORD);

 //Your whitlelist of order bys.
 $order_whitelist = array("publicationDate DESC", "publicationDate ASC", etc..);

 // see if we have such a name, if it is not in the array then $order_check will be false.
 $order_check = array_search($order, $order_whitelist); 

if ($order_check !== FALSE)
 {

 $sql = "SELECT SQL_CALC_FOUND_ROWS *, UNIX_TIMESTAMP(publicationDate) AS publicationDate FROM articles
    ORDER BY " . $order . " LIMIT :numRows";

 $st = $conn->prepare($sql);
 $st->bindValue(":numRows", $numRows, PDO::PARAM_INT);
 $st->execute();
 $list = array();

 while ($row = $st->fetch())
     {
     $article = new Article($row);
     $list[] = $article;
     }
 }

According to comment 24-Jun-17 02:42 (http://www.elated.com/articles/cms-in-an-afternoon-php-mysql/) do you recommend something like that : `$orders = ["name","price","qty"]; $key = array_search($_GET['sort'],$orders); $orderby = $orders[$key]; $query = "SELECT * FROM `table` ORDER BY $orderby";` — Alexandre Roux, Aug 05 '17 at 12:04
@AlexandreRoux yes I do, however, I would check that `$key` is set before allowing the query to be prepared and executed. — MinistryOfChaps, Aug 05 '17 at 12:07
@AlexandreRoux I have included the article's comment code in my solution so you understand what is being done. — MinistryOfChaps, Aug 05 '17 at 12:17
It work like a charm (I just noticed you missed ";" at the end of `$order_whitelist = array("publicationDate DESC", "publicationDate ASC", etc..)` that will end up as the array to able to initialize. It help me a lot to understand why a whitelist is needed here in order to avoid injection. Thanks a lot for your time and explanation. +1 — Alexandre Roux, Aug 05 '17 at 22:46

How to replace mysql_escape_string by PDO::quote()

1 Answers1