4

I'm just migrating my code from mysql_query style commands to PDO style and I ran into a problem. THe old code looked like this :

$query_list_menu = "SELECT ".$_GET['section_name']." from myl_menu_hide_show WHERE id='".$_GET['id']."'";

And the updated code looks like below. Apparently it's not working. I store in $_GET['section_name'] a string that represents a field name from the database. But I think there is a problem when I pass it as a variable. Is the below code valid ? Thanks.

$query_list_menu = "SELECT :section_name from myl_menu_hide_show WHERE id=:id";
$result_list_menu = $db->prepare($query_list_menu);
$result_list_menu->bindValue(':section_name', $_GET['section_name'] , PDO::PARAM_STR);
$result_list_menu->bindValue(':id', $_GET['id'] , PDO::PARAM_INT);  
$result_list_menu->execute();
Taz
  • 3,718
  • 2
  • 37
  • 59
Adrian Tanase
  • 671
  • 8
  • 20

2 Answers2

11

If $_GET['section_name'] contains a column name, your query should be:

$query_list_menu = "SELECT " . $_GET['section_name'] . " from myl_menu_hide_show WHERE id=:id";

Giving:

$query_list_menu = "SELECT :section_name from myl_menu_hide_show WHERE id=:id";
$result_list_menu = $db->prepare($query_list_menu);
$result_list_menu->bindValue(':id', $_GET['id'] , PDO::PARAM_INT);  
$result_list_menu->execute();

The reason is that you want the actual name of the column to be in the query - you'd changed it to be a parameter, which doesn't really make much sense.

I'll also add that using $_GET['section_name'] directly like this is a massive security risk as it allows for SQL injection. I suggest that you validate the value of $_GET['section_name'] by checking it against a list of columns before building and executing the query.

Phil
  • 2,392
  • 18
  • 21
  • 2
    An additional +1 for suggesting checking. – dezso Jul 06 '12 at 09:11
  • thanks a lot Phil.....i did how you pointed out and also executed the db UPDATE and basic SELECT code *ONLY* if the $_GET['section_name'] variable will check against 12 column names i'm using in this table....to be sure no SQL injection occurs :) thanks again for pointing out my error. have a great day – Adrian Tanase Jul 06 '12 at 09:20
0

There is no good and safe way to select just one field from the record based on the user's choice. The most sensible solution would be to select the whole row and then return the only field requested

$sql = "SELECT * from myl_menu_hide_show WHERE id=?";
$stmt = $db->prepare($query_list_menu);
$stmt->execute([$_GET['id']]);
$row = $stmt->fetch();
return $row[$_GET['section_name']] ?? false;
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345