1

I am trying to set up PHP queries for MySQL in a way to prevent SQL injection (standard website).

I had a couple of INSERT queries where changing this worked well but on the following SELECT I keep getting an error since the update and it looks like the while loop doesn't work with the changes I made (it works well without using the statement as in the old code).

Can someone tell me what I am doing wrong here ?

New PHP:

$stmt = $conn->prepare("SELECT ? FROM TranslationsMain WHERE location LIKE '%calendar weekday%' ORDER BY sortOrder, ?");
$stmt->bind_param('s', $selectedLang);
$stmt->execute();
$result = $stmt->get_result();  
while($arrCalWeekdays = $result->fetch_assoc()){
    $calWeekdays[] = $arrCalWeekdays;
}
$conn->close();

Old PHP (changed part):

$sql = "SELECT " . $selectedLang . " FROM TranslationsMain WHERE location LIKE '%calendar weekday%' ORDER BY sortOrder, " . $selectedLang;
$result = $conn->query($sql);  
while($arrCalWeekdays = $result->fetch_assoc()){
    $calWeekdays[] = $arrCalWeekdays;
}
$conn->close();

Error message:

Fatal error: Call to a member function fetch_assoc() on a non-object in /homepages/21/d580042014/htdocs/myform.php on line 21

Many thanks in advance.

Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
TaneMahuta
  • 367
  • 3
  • 8
  • 17
  • Consult http://stackoverflow.com/questions/11312737/can-i-parameterize-the-table-name-in-a-prepared-statement as to *what you cannot do* with prepared statements. – Funk Forty Niner Jun 22 '15 at 12:21
  • Where does the value of `$selectedLang` come from? Is it user-defined? – Gumbo Jun 23 '15 at 05:03

1 Answers1

5

You cannot bind column and table names, only data. You need to specify the table and then bind for your '%calendar weekday%'.

$stmt = $conn->prepare("SELECT " . $selectLang . " FROM `TranslationsMain` WHERE `location` LIKE ? ORDER BY `sortOrder`, " . $selectedLang);
$stmt->bind_param('s', $calendar_weekday);

If you want to use dynamic table / column names you should perform the minimal white-listing of those items. You can build a dynamic white list by asking the database what columns are valid for a given database table. For example:

SELECT `COLUMN_NAME` 
FROM `INFORMATION_SCHEMA`.`COLUMNS` 
WHERE `TABLE_SCHEMA` = `database_name`
AND `TABLE_NAME` = `table_name`

You could place all of this information in arrays and then check to make sure the table / column names used in the query are in the arrays. Extra consideration for table and column names should be performed, making sure that no key / reserved words are used for these names.

Finally, use backticks around the validated table / column names when calling the values for the dynamic queries. This will cover any potential changes to the key / reserved words list and provides an additional layer of protection.

Jay Blanchard
  • 34,243
  • 16
  • 77
  • 119
  • 1
    Simple yet effective answer. *Way to go Sam* – Funk Forty Niner Jun 22 '15 at 12:18
  • Thanks for this ! Can you show me what I have to do here exactly since I am new to this ? Also, for simple Selects like this where the selected column only defines a different language / translation do I even have to set it up for injection prevention ? – TaneMahuta Jun 22 '15 at 12:20
  • 1
    Binding the variables is what prevents the SQL ijnection as that is where any attempted injection would come from. I updated my answer @TaneMahuta – Jay Blanchard Jun 22 '15 at 12:21
  • 1
    *Thanks Ralph.* I'm a fairly simple guy @Fred-ii- ;-) – Jay Blanchard Jun 22 '15 at 12:22
  • @JayBlanchard: Thanks a lot for the update ! This is a big help. Just one final question: I see that you don't bind the language column but this is defined by the user. That's why I tried to bind $selectedLang as this is a variable that defines the name of the selected language column. For example: If the user selects English than $selectedLang = en which is also the column name and the result is English translations. – TaneMahuta Jun 22 '15 at 13:00
  • 1
    You cannot bind a column, so if you need to you can do the variable substitution as you originally did with `$selectedLang` @TaneMahuta. There will be no SQL injection issue from doing that. – Jay Blanchard Jun 22 '15 at 13:01
  • @JayBlanchard: Thanks again ! Got it - that helped a lot. Sorry for all the questions but this is all new to me and I just want to do it right from the start. :) – TaneMahuta Jun 22 '15 at 13:28
  • No worries! Good luck @TaneMahuta – Jay Blanchard Jun 22 '15 at 13:43
  • Why use parameterization for a static value? – Gumbo Jun 23 '15 at 05:03
  • You wouldn't have to @Gumbo, but it doesn't hurt to get in the habit. – Jay Blanchard Jun 23 '15 at 11:20
  • It’s more important to explain how to handle `$selectedLang` properly. Like applying white-listing. – Gumbo Jun 23 '15 at 16:56