Issue status update for http://drupal.org/node/27864 Post a follow up: http://drupal.org/project/comments/add/27864 Project: Drupal Version: 4.6.2 Component: node.module Category: bug reports Priority: normal Assigned to: willmoy Reported by: willmoy Updated by: willmoy Status: patch (code needs review) Attachment: http://drupal.org/files/issues/27864-node.module-4.6.2_0.patch (682 bytes) Thanks very much killes, that was dumb of me. New patch attached. willmoy Previous comments: ------------------------------------------------------------------------ Sat, 30 Jul 2005 19:58:15 +0000 : willmoy To reproduce: - Take a page which is denied to anonymous users by node_privacy_byrole - Go to it as an anonymous user - Receive 404 error Note: this bug did not exist in 4.5.x ------------------------------------------------------------------------ Sat, 30 Jul 2005 20:23:02 +0000 : willmoy Attachment: http://drupal.org/files/issues/27864-user.module-4.6.2.patch (686 bytes) Tested patch against 4.6.2 branch attached. ------------------------------------------------------------------------ Sun, 31 Jul 2005 01:01:08 +0000 : willmoy Attachment: http://drupal.org/files/issues/27864-node.module-4.6.2.patch (682 bytes) New patch. Correctly handles both 403s and 404s. Adds an extra query to verify which is happening. ------------------------------------------------------------------------ Sun, 31 Jul 2005 10:18:30 +0000 : Dries That code is insecure and may lead to SQL injection attacks. ------------------------------------------------------------------------ Sun, 31 Jul 2005 19:09:15 +0000 : willmoy The patch doesn't include enough context to show that the query is subject to a check of is_numeric(arg(1)). I don't think this is vulnerable to SQL injection, but I'd be grateful if someone else would check and I apologise if I'm missing something. The patched code in context: case 'view':       if (is_numeric(arg(1))) {         $node = node_load(array('nid' => arg(1)), $_GET['revision']);         if ($node->nid) {           drupal_set_title(check_plain($node->title));           print theme('page', node_show($node, arg(2)));         }         else if (db_fetch_object(db_query('SELECT nid FROM {node} WHERE nid=%s', arg(1)))) {           drupal_access_denied();         }         else {           drupal_not_found();         }       }       break; The cvs version of this patch http://drupal.org/node/27873 has the same context. ------------------------------------------------------------------------ Sun, 31 Jul 2005 19:42:45 +0000 : killes@www.drop.org I think the problem is this: WHERE nid=%s', arg(1) It should be WHERE nid = '%s'", arg(1) if arg(1) were indeed a string, but since it isn't it should be WHERE nid = %d', arg(1)