Search code examples
phpmysqldatabasesql-injection

Is there anything that can be put after the "ORDER BY" clause that can pose a security risk?


Basically, what I want to do is this:

mysql_query("SELECT ... FROM ... ORDER BY $_GET[order]")

They can obviously easily create a SQL error by putting non-sense in there, but mysql_query only allows you to execute 1 query, so they can't put something like 1; DROP TABLE ....

Is there any damage a malicious user could do, other than creating a syntax error?

If so, how can I sanitize the query?

There's a lot of logic built on the $_GET['order'] variable being in SQL-like syntax, so I really don't want to change the format.


To clarify, $_GET['order'] won't just be a single field/column. It might be something like last_name DESC, first_name ASC.


Solution

  • Yes, SQL injection attacks can use an unescaped ORDER BY clause as a vector. There's an explanation of how this can be exploited and how to avoid this problem here:

    http://josephkeeler.com/2009/05/php-security-sql-injection-in-order-by/

    That blog post recommends using a white list to validate the ORDER BY parameter against, which is almost certainly the safest approach.


    To respond to the update, even if the clause is complex, you can still write a routine that validates it against a whitelist, for example:

    function validate_order_by($order_by_parameter) {
        $columns = array('first_name', 'last_name', 'zip', 'created_at');
    
        $parts = preg_split("/[\s,]+/", $order_by_parameter);
    
        foreach ($parts as $part) {
            $subparts = preg_split("/\s+/", $part);
    
            if (count($subparts) < 0 || count($subparts) > 2) {
               // Too many or too few parts.
               return false;
            }
    
            if (!in_array($subparts[0], $columns)) {
               // Column name is invalid.
               return false;
            }
    
            if (count($subparts) == 2 
                && !in_array(strtoupper($subparts[1]), array('ASC', 'DESC')) {
              // ASC or DESC is invalid
              return false;
            }
        }
    
        return true;
    }
    

    Even if the ORDER BY clause is complex, it's still made only out of values you supply (assuming you're not letting users edit it by hand). You can still validate using a white list.

    I should also add that I normally don't like to expose my database structure in URLs or other places in the UI and will often alias the stuff in the parameters in the URLs and map it to the real values using a hash.