Author Topic: [2.0.15] Occasional PHP Notices in allowedTo()  (Read 1519 times)

Offline MrManager

  • Semi-Newbie
  • *
  • Posts: 36
[2.0.15] Occasional PHP Notices in allowedTo()
« on: January 16, 2018, 05:17:45 PM »
I use SMF 2.0.15 through SSI.php. Sometimes, I see these PHP warnings in my error log:

Code: [Select]
PHP Notice:  Undefined index: is_admin in /forum/Sources/Security.php on line 824
PHP Notice:  Undefined index: permissions in /forum/Sources/Security.php on line 831
PHP Warning:  in_array() expects parameter 2 to be array, null given in /forum/Sources/Security.php on line 831
PHP Notice:  Undefined index: id in /forum/Sources/Load.php online 1578
PHP Notice:  Undefined index: is_guest in /forum/Sources/Load.php on line 1579

As well as:
Code: [Select]
PHP Notice:  Undefined index: language in /forum/Sources/Load.php on line 1747
After seeing this for a few years I finally got fed up and added some logging to track this down.

It looks like this is because the user_info array used by allowedTo() looks like this, lacking many of the usual values (like is_admin, permissions, etc):
Code: [Select]
Or like this:
Code: [Select]
The call to allowedTo seems to sometimes (but not always) come from this line in Load.php:
Code: [Select]
if (allowedTo('admin_forum') && isset($_REQUEST['debug']) && !in_array($sub_template_name, array('init', 'main_below')) && ob_get_length() > 0 && !isset($_REQUEST['xml']))
Or this:
Code: [Select]
elseif (!empty($modSettings['knownThemes']) && !allowedTo('admin_forum'))

Which is in turn called from fatal_lang_error() in Errors.php, which is called from validatePasswordFlood() in LogInOut.php:727

I can't really figure out how this could happen (it does not seem to occur if I just enter a wrong password and trigger flood control) but for some reason in these cases user_info is not set up properly which trips up allowedTo().

I think a simple way to get rid of some of the warnings would be to, in Security.php below this:
Code: [Select]
// You're never allowed to do something if your data hasn't been loaded yet!
if (empty($user_info))
return false;
Add this:

Code: [Select]
// If permissions have not been set up properly, return false
if (!isset($user_info['permissions']))
return false;

However, you may need to check that this doesn't happen under normal circumstances, ie. that $user_info['permissions'] is normally guaranteed to be set.

A cleaner way would be to make sure that user_info is always either set up properly or set to null, to make sure the empty($user_info) check catches it already. However, I am not sure where this fix would have to be made in this case.

For now, I will just manually patch this in my installation, but would be great if this could get added to the next version!