错误! mysqli_escape_string()

I'm fairly new to php and trying out some "tutorials" floating around on the web. One problem I have run into is older tutorials on "up-to-date" php/MySQL platforms.

For instance:

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

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

Because mysql_escape.. is deprecated, I'm trying to replace it with mysqli_escape_string($conn, $order). But i believe my $st = $conn->prepare($sql) is throwing monkey wrench in the mix.

Any suggestions/explanations?

You're using PDO, so you should be using prepared queries anyway.

That said, prepared queries won't work for what you're trying to do. And in any case, mysql_real_escape_string WILL NOT SAVE YOU HERE.

Instead, you could try some custom escaping:

"... ORDER BY `".str_replace(array("`","\\"),"",$order)."` LIMIT ...";

Backticks are USEFUL! :p

Okay several issues:

  • You can't mix mysql_* functions with PDO. I know it's confusing, because PHP has three different API's for MySQL. But you have to use one at a time.

  • The escaping functions, including PDO::quote(), are only for escaping string literals and date literals. They don't do the right thing for column names.

  • Your $order variable has two syntax elements: the column, and the direction. Even if you were to quote the column properly, you wouldn't want to include DESC inside the quotes.

  • The direction (ASC/DESC) is an SQL keyword, so it does not go into quotes. Therefore there's no escaping possible to make it safe against SQL injection.

My solution to this is not to use quoting/escaping, but to use whitelisting. And keep the sort column separate from the sort direction.

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

    $columns = array("publicationDate"=>"publicationDate", "name"=>"authorName", "DEFAULT"=>"publicationDate");
    $directions = array("up"=>"ASC", "down"=>"DESC", "DEFAULT"=>"DESC");

    $order_column    = $columns[$order] ?: $columns["DEFAULT"];
    $order_direction = $directions[$direction] ?: $directions["DEFAULT"];

    $conn = new PDO( DB_DSN, DB_USERNAME, DB_PASSWORD );
    $sql = "SELECT *, UNIX_TIMESTAMP(publicationDate) AS publicationDate 
        FROM articles
        ORDER BY $order_column $order_direction LIMIT :numRows";
    . . .
 }

Tip: I never use SQL_CALC_FOUND_ROWS because it is such a dire performance killer. It's actually much better to run two queries, one to get a SELECT COUNT(*)... and the second to get the limited result set.
See tests at To SQL_CALC_FOUND_ROWS or not to SQL_CALC_FOUND_ROWS?