- sa-2006-001: Custom menu items are accessible to anyone

includes/menu.inc

@@ -1073,7 +1073,7 @@ function _menu_build() {
         else {
           // It has a permanent ID. Only replace with non-custom menu items.
           if ($item->type & MENU_CREATED_BY_ADMIN) {
-            $_menu['items'][$item->mid] = array('path' => $item->path, 'access' => TRUE);
+            $_menu['items'][$item->mid] = array('path' => $item->path);
           }
           else {
             // Leave the old item around as a shortcut to this one.
@@ -1085,7 +1085,7 @@ function _menu_build() {
       else {
         // The path was not declared, so this is a custom item or an orphaned one.
         if ($item->type & MENU_CREATED_BY_ADMIN) {
-          $_menu['items'][$item->mid] = array('path' => $item->path, 'access' => TRUE);
+          $_menu['items'][$item->mid] = array('path' => $item->path);
           if (!empty($item->path)) {
             $_menu['path index'][$item->path] = $item->mid;
           }

modules/user.module

@@ -686,8 +686,7 @@ function user_menu($may_cache) {
 
   $admin_access = user_access('administer users');
   $access_access = user_access('administer access control');
-  // Users should always be allowed to see their own user page
+  $view_access = user_access('access user profiles');
-  $view_access = (user_access('access user profiles') || ($user->uid == arg(1)));
 
   if ($may_cache) {
     $items[] = array('path' => 'user', 'title' => t('user account'),
@@ -769,15 +768,21 @@ function user_menu($may_cache) {
   }
   else {
     if (arg(0) == 'user' && is_numeric(arg(1))) {
-      $user_exists = user_load(array('uid' => arg(1), 'status' => 1));
+      $account = user_load(array('uid' => arg(1)));
+
+      if ($user !== FALSE) {
+        // Always let a user view their own account
+        $view_access |= $user->uid == arg(1);
+        // Only admins can view blocked accounts
+        $view_access &= $account->status || $admin_access; 
 
         $items[] = array('path' => 'user/'. arg(1), 'title' => t('user'),
           'type' => MENU_CALLBACK, 'callback' => 'user_view',
           'callback arguments' => array(arg(1)), 'access' => $view_access);        
 
-      if ($user_exists !== FALSE || $admin_access) {
         $items[] = array('path' => 'user/'. arg(1) .'/view', 'title' => t('view'),
           'access' => $view_access, 'type' => MENU_DEFAULT_LOCAL_TASK, 'weight' => -10);
+
         $items[] = array('path' => 'user/'. arg(1) .'/edit', 'title' => t('edit'),
           'callback' => 'user_edit', 'access' => $admin_access || $user->uid == arg(1),
           'type' => MENU_LOCAL_TASK);
@@ -1401,7 +1406,7 @@ function user_edit_submit($form_id, $form_values) {
 function user_view($uid = 0) {
   global $user;
 
-  if ($account = user_load(array('uid' => $uid, 'status' => 1))) {
+  $account = user_load(array('uid' => $uid));
   // Retrieve and merge all profile fields:
   $fields = array();
   foreach (module_list() as $module) {
@@ -1417,10 +1422,6 @@ function user_view($uid = 0) {
   drupal_set_title($account->name);
   return theme('user_profile', $account, $fields);
 }
-  else {
-    drupal_not_found();
-  }
-}
 
 /*** Administrative features ***********************************************/
 

modules/user/user.module

@@ -686,8 +686,7 @@ function user_menu($may_cache) {
 
   $admin_access = user_access('administer users');
   $access_access = user_access('administer access control');
-  // Users should always be allowed to see their own user page
+  $view_access = user_access('access user profiles');
-  $view_access = (user_access('access user profiles') || ($user->uid == arg(1)));
 
   if ($may_cache) {
     $items[] = array('path' => 'user', 'title' => t('user account'),
@@ -769,15 +768,21 @@ function user_menu($may_cache) {
   }
   else {
     if (arg(0) == 'user' && is_numeric(arg(1))) {
-      $user_exists = user_load(array('uid' => arg(1), 'status' => 1));
+      $account = user_load(array('uid' => arg(1)));
+
+      if ($user !== FALSE) {
+        // Always let a user view their own account
+        $view_access |= $user->uid == arg(1);
+        // Only admins can view blocked accounts
+        $view_access &= $account->status || $admin_access; 
 
         $items[] = array('path' => 'user/'. arg(1), 'title' => t('user'),
           'type' => MENU_CALLBACK, 'callback' => 'user_view',
           'callback arguments' => array(arg(1)), 'access' => $view_access);        
 
-      if ($user_exists !== FALSE || $admin_access) {
         $items[] = array('path' => 'user/'. arg(1) .'/view', 'title' => t('view'),
           'access' => $view_access, 'type' => MENU_DEFAULT_LOCAL_TASK, 'weight' => -10);
+
         $items[] = array('path' => 'user/'. arg(1) .'/edit', 'title' => t('edit'),
           'callback' => 'user_edit', 'access' => $admin_access || $user->uid == arg(1),
           'type' => MENU_LOCAL_TASK);
@@ -1401,7 +1406,7 @@ function user_edit_submit($form_id, $form_values) {
 function user_view($uid = 0) {
   global $user;
 
-  if ($account = user_load(array('uid' => $uid, 'status' => 1))) {
+  $account = user_load(array('uid' => $uid));
   // Retrieve and merge all profile fields:
   $fields = array();
   foreach (module_list() as $module) {
@@ -1417,10 +1422,6 @@ function user_view($uid = 0) {
   drupal_set_title($account->name);
   return theme('user_profile', $account, $fields);
 }
-  else {
-    drupal_not_found();
-  }
-}
 
 /*** Administrative features ***********************************************/