Is this a safe way of using HTTP query parameters to build a SQL query?

Question

Is it? And maybe theres a better way to do this?

$allowed = array("name_desc", "name_asc", "level_desc", "level_asc", "vocation_desc", "vocation_asc");
$order = isset($_GET['order']) ? $_GET['order'] : "name";
$order = in_array($order, $allowed) ? str_replace("_", " ", $order) : "name";
$query = "SELECT * FROM players 
 WHERE status = 0 AND group_id < 3 ORDER BY $order";

Answer 1

It is safe. You have a white list of allowed values and ensure that the user provided input is on the list.

But I'd still do the white list checking as the very first thing, and then do the other processing later, so it would be:

$field = 'name';
$direction = 'asc';
if(preg_match('^([a-z]+)_(asc|desc)$', $_GET['order'], $matches)) {
 if(in_array($matches[0], array("name", "level", "vocation"))) {
 $field = $matches[0];
 $direction = $matches[1];
 }
}

What happens here is that the block splits up and validates the input. You can be sure that once you're past this block, $field and $direction can be trusted. An important point is to use $_GET['order'] exactly once, since accessing it multiple times may lead to errors that will leak non-validated data.

• remy: So I dont have to escape the string before using it in a query?
• Michael Zedeler: No. You have just made sure it contains nothing dangerous. Escaping wouldn't change the contents of the string anyway.