Search code examples
phpmysqlpdoprepared-statementsql-injection

How to replace mysql_escape_string by PDO::quote()


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;
}

Solution

  • 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;
         }
     }