Take these two lines from lines 369 and 370 of tac_lite.module

    $join = "LEFT JOIN {term_data} catd ON $primary_table.tid = catd.tid";
    $where = "$primary_table.tid IN (". implode(', ', $tids) .") OR catd.vid NOT IN (". implode(',', $vids) .")";

Two major problems here. One is that you have put variables into an SQL statement without using the percent modifier syntax, this leaves your SQL prone to injection attacks. Two is that this statement causes SQL to break and hurl error message at the Drupal user when there are no Categories and/or Vocabularies set up. I know it's a taxonomy module - but you should handle these situations anyway :P

user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ')) AND ( r.nid = 1 ) ORDER BY v.weight, t.weight, t.name' at line 1 query: SELECT t.* FROM term_node r INNER JOIN term_data t ON r.tid = t.tid INNER JOIN vocabulary v ON t.vid = v.vid LEFT JOIN term_data catd ON t.tid = catd.tid WHERE (t.tid IN (0) OR catd.vid NOT IN ()) AND ( r.nid = 1 ) ORDER BY v.weight, t.weight, t.name in /home/svn/codebase/drupal/ecommerce-5.6/includes/database.mysql.inc on line 172.

Comments

danielb’s picture

danielb commented

I don't know exactly what you are doing with this query, so I'm not going to provide the answer for how this code should be. But just so you know what I meant about the percent modifiers here is an example

$result = db_query("SELECT * FROM {women} WHERE role IN ('%s')", implode(", ", $your_mum))

you could easily incorporate that condition ? result : default syntax to plug in a dummy value to the sql which will stop the error and still result in the same results set..

Dave Cohen’s picture

Dave Cohen commented
Status: Active » Closed (duplicate)

http://drupal.org/node/230952

You're right that when calling db_query it's best to use the percent modifiers. In this case the code never calls db_query, so it never has a chance to use percent modifiers. It's not a problem because the variables all come from values queried from the database. There's nothing passed in from a user here. Only someone with access to the database could do something malicious.

This code ensures that when drupal displays a list of vocabulary terms, it does not include those the user is not allowed to see.