Skip to content

Commit

Permalink
Only caching access lists after ready, system fires.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
brettp committed May 15, 2012
1 parent 70e5ffe commit 9a59aa7
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 19 deletions.
16 changes: 9 additions & 7 deletions CHANGES.txt
Expand Up @@ -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
Expand Down
31 changes: 19 additions & 12 deletions engine/lib/access.php
Expand Up @@ -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();
}

Expand All @@ -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;
}
}

/**
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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];
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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');
Expand Down

0 comments on commit 9a59aa7

Please sign in to comment.