4.6.2 node.module incorrectly assumes 404 when access denied [#27864]

Comment	File	Size	Author
#6	27864-node.module-4.6.2_0.patch	682 bytes	willmoy

#2	27864-node.module-4.6.2.patch	682 bytes	willmoy

#1	27864-user.module-4.6.2.patch	686 bytes	willmoy

Comment #1

willmoy CreditAttribution: willmoy commented 30 July 2005 at 20:23

Title:	user.module incorrectly assumes 404 when access denied	» node.module incorrectly assumes 404 when access denied
Component:	user.module	» node.module

File	Size
27864-user.module-4.6.2.patch	686 bytes

Tested patch against 4.6.2 branch attached.

Log in or register to post comments

Comment #2

willmoy CreditAttribution: willmoy commented 31 July 2005 at 01:01

Title:	node.module incorrectly assumes 404 when access denied	» 4.6.2 node.module incorrectly assumes 404 when access denied
Status:	Active	» Needs review

File	Size
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.

Log in or register to post comments

Comment #3

Dries CreditAttribution: Dries commented 31 July 2005 at 10:18

Status:

Needs review

» Needs work

That code is insecure and may lead to SQL injection attacks.

Log in or register to post comments

Comment #4

willmoy CreditAttribution: willmoy commented 31 July 2005 at 19:09

Status:

Needs work

» Needs review

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.