1

I am using MeekroDB in a PHP project. For some queries, I need to pass arbitrary field names to sort by. There are NO examples of ORDER BY clauses on the meekro site.

How can I safely pass field names and avoid SQL injection vulnerabilities? I realize I could check every field name with a list of valid fields beforehand, but I'm trying to make this code more generalized as a basic "get" function: function get(Array $filters, Array $sort_by)

Will the %b placeholder (backticks) be sufficient to protect against arbitrary code injection when passing field names?

For example: 

SELECT * FROM table1 ORDER BY %b

Or for multiple fields: 

SELECT * FROM table1 ORDER BY %lb

Is this safe?

Also, how can I then include the DESC or ASC modifiers arbitrarily as needed?

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
Ryan Griggs
  • 2,457
  • 2
  • 35
  • 58

1 Answers1

1

Yes, you can safely use b and lb for the purpose, as both implemented using formatTableName method that is safe.

Unfortunately, direction modifiers should be sanitized by hand, like this 

$dirs  = ["ASC","DESC"]; 
$key   = array_search($_GET['dir'], $dirs); // see if we have such a value
$dir   = $dirs[$key]; //if not, first one will be set automatically. smart enuf :)
$query = "SELECT * FROM table1 ORDER BY %b $dir"; //value is safe
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • Thanks for your answer! How would I add multiple ORDER BY fields, each with its own direction modifier? For example SELECT * FROM table1 ORDER BY `field1` DESC, `field2` ASC? How would the actual meekro DB::query() function look? (I realize I can iterate through the order fields and add a %b %l for each 'field' and 'direction' specifier. But I can't understand how to dynamically pass these to the DB::query function. – Ryan Griggs Oct 13 '16 at 15:45
  • That will be a hard task. What I would suggest is to open an issue on the Meekro github, describing a situation and suggesting that both b and lb modifiers can accept an array consists of a field name and direction. Or you can go with your own suggestion with separate `b`s. I shouldn't be very complex. – Your Common Sense Oct 13 '16 at 15:53
  • Using separate %b and %l would require that an arbitrary number of parameters be passed to the DB::query () . I would have to call it using eval which I don't feel is a good practice. Any suggestions? – Ryan Griggs Oct 13 '16 at 16:15
  • 1
    No need for eval ever. Since PHP 5.6 there is a great *splat* operator, when you can send an array for a function that accepts a list of parameters, just like that: `$results = DB::query($sql, ...$parameters);` If your PHP is outdated, then you have to stick with `call_user_func_array()` – Your Common Sense Oct 13 '16 at 16:19
  • Nice. I didn't know about that operator! Perfect. – Ryan Griggs Oct 13 '16 at 19:31