Only caching access lists after ready, system fires.

This prevents a bug where access lists could be cached
and not cleared during plugin boot while access was disabled,
which could expose entities set to ACCESS_PRIVATE.

diff --git a/CHANGES.txt b/CHANGES.txt
index f5cacac29dd4464882f03a6eebc6da2f322e8329..ae0cdc333c858a301518ad5eef3501bf413d699d 100644 (file)
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -8,17 +8,19 @@ Version 1.8.5
  Security Enhancements:
   * Fixed possible XSS vulnerability if using a crafted URL.
   * Fixed exploit to bypass new user validation if using a crafted form.
+  * Fixed incorrect caching of access lists that could allow plugins
+    to show private entities to non-admin and non-owning users. (Non-exploitable)
 
  Bugfixes:
-   * Twitter API: New users are forwarded to the correct page after creating 
-                  an account with Twitter.
-   * Files: PDF files are downloaded as "inline" to display in the browser.
-   * Fixed possible duplication errors when writing metadata with multiple values.
-   * Fixed possible upgrade issue if using a plugin uses the system_log hooks.
-   * Fixed problems when enabling more than 50 metadata or annotations.
+  * Twitter API: New users are forwarded to the correct page after creating 
+                 an account with Twitter.
+  * Files: PDF files are downloaded as "inline" to display in the browser.
+  * Fixed possible duplication errors when writing metadata with multiple values.
+  * Fixed possible upgrade issue if using a plugin uses the system_log hooks.
+  * Fixed problems when enabling more than 50 metadata or annotations.
 
  API:
-   * River entries' timestamps use elgg_view_friendly_time() and can be 
+  * River entries' timestamps use elgg_view_friendly_time() and can be 
      overridden with the friendly time output view.
 
 Version 1.8.4

diff --git a/engine/lib/access.php b/engine/lib/access.php
index 6be252c6a33a8ef233677cc8da9f1c5601724038..702e2c1cbb3f27b26da6150dd414d8fcf7897daf 100644 (file)
--- a/engine/lib/access.php
+++ b/engine/lib/access.php
@@ -31,7 +31,7 @@ function get_access_list($user_id = 0, $site_id = 0, $flush = false) {
        global $CONFIG, $init_finished;
        static $access_list;
 
-       if (!isset($access_list) || !$init_finished) {
+       if (!isset($access_list)) {
                $access_list = array();
        }
 
@@ -49,9 +49,15 @@ function get_access_list($user_id = 0, $site_id = 0, $flush = false) {
                return $access_list[$user_id];
        }
 
-       $access_list[$user_id] = "(" . implode(",", get_access_array($user_id, $site_id, $flush)) . ")";
+       $access = "(" . implode(",", get_access_array($user_id, $site_id, $flush)) . ")";
 
-       return $access_list[$user_id];
+       // only cache if done with init
+       if ($init_finished) {
+               $access_list[$user_id] = $access;
+               return $access_list[$user_id];
+       } else {
+               return $access;
+       }
 }
 
 /**
@@ -83,7 +89,7 @@ function get_access_array($user_id = 0, $site_id = 0, $flush = false) {
        // this cache might be redundant. But db cache is flushed on every db write.
        static $access_array;
 
-       if (!isset($access_array) || (!isset($init_finished)) || (!$init_finished)) {
+       if (!isset($access_array)) {
                $access_array = array();
        }
 
@@ -137,12 +143,11 @@ function get_access_array($user_id = 0, $site_id = 0, $flush = false) {
                                $tmp_access_array[] = ACCESS_PRIVATE;
                        }
 
-                       $access_array[$user_id] = $tmp_access_array;
-               } else {
-                       // No user id logged in so we can only access public info
-                       $tmp_return = $tmp_access_array;
+                       // only cache if done with init
+                       if ($init_finished) {
+                               $access_array[$user_id] = $tmp_access_array;
+                       }
                }
-
        } else {
                $tmp_access_array = $access_array[$user_id];
        }
@@ -946,7 +951,8 @@ function elgg_get_access_object() {
  *
  * @global bool $init_finished
  * @access private
- * @todo investigate why this is needed
+ * @todo This is required to tell the access system to start caching because
+ * calls are made while in ignore access mode and before the user is logged in.
  */
 $init_finished = false;
 
@@ -1014,8 +1020,9 @@ function access_test($hook, $type, $value, $params) {
        return $value;
 }
 
-// This function will let us know when 'init' has finished
-elgg_register_event_handler('init', 'system', 'access_init', 9999);
+// Tell the access functions the system has booted, plugins are loaded,
+// and the user is logged in so it can start caching
+elgg_register_event_handler('ready', 'system', 'access_init');
 
 // For overrided permissions
 elgg_register_plugin_hook_handler('permissions_check', 'all', 'elgg_override_permissions');