Spot the difference
Submitted by AjK on 11 December 2008 - 11:31pm
This cracker came in recently, can you spot the difference? Which is right and which is wrong?
- db_query("DELETE FROM {$node->type} WHERE vid = %d", $node->vid);
- db_query("DELETE FROM {foo_bar_baz} WHERE vid = %d", $node->vid);
Got it yet? You may have easily spotted the first one was wrong but did you get why it's wrong? It's bad on more levels than you may at first think.
The first problem simply is burying any variable in SQL is a bad idea. Generally speaking it's only user input variables that of real concern as that's the SQL Injection attack vector. So, let us suppose this author knows what they are doing and a variable table name is fine. There's still a problem.
In the second SQL statement the constant table name is enclosed in curly braces. Drupal's DB Abstraction layer likes this, it allows for Drupal to use table name prefixing. But what's that in the first line? There sure is curly braces but Drupal's API will never see them. Because the string is double quoted PHP will nip in there and strip them assuming you're embedding a complex variable. So the first line of code doesn't have Drupal's {...} requirement for prefixing tables.
