Cracking Drupal®: A Drop in the Bucket

10.3. Step 3: Secure the Node List

The node-list feature the Vulnerable module has several major XSS and SQL injection problems. This page provides two features:

• It can be accessed with a number in the URL, in which case it will load that node and display it.

• If it is accessed without any additional arguments, it will simply display a list of all the nodes on the site.

This presents several problems. First things first, though. The single case:

$node = node_load(arg(2));
$access = db_result(db_query("SELECT n.nid FROM {node} n WHERE n.nid
  = $node->nid"));
if ($access) {
  drupal_set_message($node->title);
}

This code is both weak to exploitation and does too much work. It would be possible to fix this code while maintaining its basic structure with the following changes:

$node = node_load(arg(2));
$access = db_result(db_query(db_rewrite_sql("SELECT n.nid FROM {node} n WHERE
  n.status = 1 AND n.nid = %d"), $node->nid));
if ($access) {
  drupal_set_message(check_plain($node->title));
}

Adding in the db_rewrite_sql, moving the query variable into a parameter, adding a check that the node is published, and adding a check_plain to the node title will all protect this code from SQL injection, access bypass, and XSS attacks. But it still does too much work. Chapter 9 showed the proper way to use node_access to reduce the effort in this example:

$node = node_load(arg(2));
if (node_access('view', $node)) {
  drupal_set_message($node->title);
}

Now for the bigger listing of nodes. Once again, you ...