1

I need to parameterize both the ORDER BY column name and DESC or ASC keyword as a variable. I have it working already but the variables are in the query, which is not safe.

$fruitColumn='bananas'
$sortbyORDER= 'DESC'
$fruitSearch = $db->prepare("
SELECT * FROM fruits
ORDER BY $fruitColumn $sortbyORDER");
$fruitSearch ->execute();

BUT - I need the variables bound in the execution for security. 

$fruitSearch = $db->prepare("
SELECT * FROM fruits
ORDER BY ? ?");
$fruitSearch ->execute([$fruitType,$sortbyORDER]);

This doesn't seem to work as the two ?s give me an error.

I have also tried combining the two variables into one eg. $order = ".$fruitColumn." ".$sortbyORDER;

Is this possible? Thanks in advance

gavin stanley
  • 1,082
  • 2
  • 13
  • 28
  • 2
    It's safe to use variables inside queries if you verify them beforehand. In this case, you know that variable can _only_ be equal to `['ASC', 'DESC']`, so just check that before the query and if it's not either of those values, stop the script. Same for columns, you (presumably) have a list of possible values. – GrumpyCrouton Jul 15 '19 at 14:14
  • Are you sure that there is a column in your table by name `bananas` ?? – Rohit Mittal Jul 15 '19 at 14:17
  • Can you point me to the answer in https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php where it answers my question please? This concerns using specifically two variables in the ORDER BY line of sql. Thanks – gavin stanley Jul 15 '19 at 14:17
  • 1
    You cannot parameterize column and table names. The only solution is to filter them out. I closed this question as a duplicate because there is my answer in the existing question, [straight on this very topic](https://stackoverflow.com/a/8255054/285587). However, on a second thought may be it would be a good idea to have a dedicated answer for this particular question – Your Common Sense Jul 15 '19 at 14:19
  • @YourCommonSense, thanks - this is specifically about making the ORDER BY parameters dynamic (and secure). Maybe I will change the question title. I cannot quite see the answer in your suggested duplicate, though the comments here are helping me. What is your suggested solution please? (I refer to your site religiously!) – gavin stanley Jul 15 '19 at 14:21
  • @YourCommonSense - would you consider unduplicating this as I still don't have an answer? Thanks – gavin stanley Jul 15 '19 at 14:26
  • if you prefer to take it from my site, here it is, [Prepared statements and table names](https://phpdelusions.net/pdo#identifiers). – Your Common Sense Jul 15 '19 at 14:48
  • Thanks @YourCommonSense, your site is awesome. – gavin stanley Jul 15 '19 at 15:17
  • You need a white-list of safe values. `if $userSubmittedData === "yes" then $data = "yes" ` sort of thing. Your Common Sense's reference to his website as well as Shaik's answer cover everything you need to build a **safe** manual SQL string. – Martin Jul 15 '19 at 18:08
  • @Martin to be honest, Shaik's answer is awfilly misleading. it suggests a different type of string concatenation as a protection measure. – Your Common Sense Jul 16 '19 at 08:46
  • @YourCommonSense it was actually that it didn't ever use the `$_GET` value in the SQL. I was only meaning the `if/else` at the top of the code block. – Martin Jul 16 '19 at 08:49
  • @Martin if else only covers the direction part whereas the field name is suggested to be added to the query as is simply using concatenation. To be honest, this answer is a great disservice. Especially if you think not only of a single OP but of many other people who might find this answer. – Your Common Sense Jul 16 '19 at 08:53
  • @YourCommonSense yes, it's only for the `ASC`/`DESC` choice but it illustrates a better method than using user-given values. I'd do a few things differently but I am not bothered enough to write an answer! `:-D` – Martin Jul 16 '19 at 09:00

0 Answers0