0

How can I replace mysql_real_escape_string in PDO to keep my code still secure:

(mysql_real_escape_string no longer supported in PHP v7.x)

I would appreciate if somebody could provide an example for this code line: "ORDER BY " . mysql_real_escape_string($order) . " LIMIT :numRows";"

Full code below:

 public static function getList( $numRows=1000000, $order="pageID ASC" ) {
    $conn = new PDO( DB_DSN, DB_USERNAME, DB_PASSWORD );
    $sql = "SELECT SQL_CALC_FOUND_ROWS *, UNIX_TIMESTAMP(pagePublicationDate) AS pagePublicationDate FROM web_pages
            ORDER BY " . mysql_real_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 cmsEngine( $row );
      $list[] = $article;
    }

    // Now get the total number of website pages that matched the criteria
    $sql = "SELECT FOUND_ROWS() AS totalRows";
    $totalRows = $conn->query( $sql )->fetch();
    $conn = null;
    return ( array ( "results" => $list, "totalRows" => $totalRows[0] ) );
  }
Tom
  • 9
  • 4
  • Not duplicated as not same question in my query.. Online examples and StackOverflow topics don't contain samples of sanitizing variables in SQL query.. (please refer to my example). – Tom Aug 26 '16 at 10:47
  • There are *different* answers. Check the [third one](http://stackoverflow.com/a/8255054/285587). – Your Common Sense Aug 26 '16 at 10:56
  • Also note that your `mysql_real_escape_string($order)` **did not sanitize anything** and your code were vulnerable. So you better not to look for replacement – Your Common Sense Aug 26 '16 at 10:58
  • Comments without facts doesn't make any sense.. Can somebody explain to me? – Tom Aug 26 '16 at 11:36
  • You can try here, [A slightly non-trivial SQL injection example. String escaping](https://phpdelusions.net/pdo/sql_injection_example#escaping) and tell me whether it's convincing. I am the author of the article and eager to get any feedback. – Your Common Sense Aug 26 '16 at 11:39
  • Great article, I have seen it before. However as I am not pro developer, I am learning by examples.. So would like to understand what is wrong with my code, am I need to backstick sql table items and etc? I was reading following: https://phpdelusions.net/pdo#group + your article – Tom Aug 26 '16 at 12:00
  • You need rather this [one](https://phpdelusions.net/pdo#identifiers). In short, you cannot sanitize an arbitrary query part like `pageID ASC` at all. It's just impossible. But you can whitelist its parts, the field name and the direction separately. – Your Common Sense Aug 26 '16 at 12:09
  • Thanks, I was looking into this. But wasn't sure how to transform my existing code to sample you supplied. Also I am reading these samples multi times and still can't understand. I have removed and mysql_real_string_escape function completely now and just quering against $orders variable. I still can't understand why my code is vulnerable - I have performed various SQL pen test against web app and none of them showed vulnerability in this particular query.. – Tom Aug 26 '16 at 12:17
  • What it is also confusing on these internet articles, that most authors provides only bits of code (I mean not giving full picture), puzzling doesn't help to understand efficiently. – Tom Aug 26 '16 at 12:21
  • Just make it `pageID ASC;DROP TABLE web_pages#` and see – Your Common Sense Aug 26 '16 at 12:46
  • Just tested with sqlmap: 14:55:18] [CRITICAL] all tested parameters appear to be not injectable. – Tom Aug 26 '16 at 12:56
  • I don't know this sqlmap thing is but if you make your $order as shown above you will see how much protection you can get from mysql_real_escape_string($order). – Your Common Sense Aug 26 '16 at 13:05
  • Is my function will be safe if I will replace $order with static item, so instead: mysql_real_escape_string($order). I would have: $sql = "SELECT SQL_CALC_FOUND_ROWS *, UNIX_TIMESTAMP(pagePublicationDate) AS pagePublicationDate FROM `web_pages` ORDER BY pageID ASC LIMIT :numRows"; – Tom Aug 26 '16 at 13:18
  • Yes, of course this way it will be secure – Your Common Sense Aug 26 '16 at 13:20
  • Oh, great! Thanks for explanation, now I understand. What about if I still want to keep it as variable - what would be safe option? Would it be possible to have an example? Maybe I should extra binding something like: $st->bindValue( ":pageID", $orders, PDO::PARAM_INT ); – Tom Aug 26 '16 at 13:35
  • No, you cannot bind anything beside a string and a nubmer. And your pageID ASC is neither. Start from keeping ASC in place while treat ordering field using examples I liked to before. – Your Common Sense Aug 26 '16 at 14:18
  • Great, what about my other variable: $numRows ? Am I right thinking that it will remain safe as it is being binded to integer? $st->bindValue( ":numRows", $numRows, PDO::PARAM_INT ); – Tom Aug 26 '16 at 15:00
  • yes, of course, that's canonical way to do that. however, you can make your code 3 times shorter. PDO can create object right off request. Basically you will need 3 commands, `$st = $conn->prepare( $sql ); $st->execute([$numRows]); $list = $st->fetchAll(PDO::FETCH_CLASS, 'cmsEngine');` – Your Common Sense Aug 26 '16 at 15:05
  • Thanks for all your explanations Sir! Appreciate your assistance and value people like you! – Tom Aug 26 '16 at 15:19
  • Ok, I will reply soon. Cleaning up the comments. – Your Common Sense Aug 31 '16 at 14:33

0 Answers0