Critical bug: AIOSEO initialize $wp_roles global variable prematurely and broke wordpress role management

[congkun-ck]

One bug was observed on a website using AIOSEO + WPForm Lite+Unlimited Member.
AIOSEO: 4.1.4.3
WPForms Lite 1.6.9
Unlimited Member: 2.2.4

Settings for custom roles created in Unlimited Member don't take effect. For example, create a custom role in Unlimited Member, and set it to hide adminbar for that role. However that setting doesn't work and adminbar will always show up for that role.

The main cause is a severe bug in the plugin loading path in AIOSEO. A WP_Roles instance was created during plugging loading. Below is the call stack:

WP_Roles::init_roles @ /wp-includes/class-wp-roles.php, Line 344
WP_Roles::for_site @ /wp-includes/class-wp-roles.php, Line 90
WP_Roles::__construct @ /wp-content/plugins/all-in-one-seo-pack/app/Common/Traits/Helpers/Wp.php, Line 132
AIOSEO\Plugin\Common\Utils\Helpers::getUserRoles @ /wp-content/plugins/all-in-one-seo-pack/app/Common/Utils/Access.php, Line 60
AIOSEO\Plugin\Common\Utils\Access::__construct @ /wp-content/plugins/all-in-one-seo-pack/app/AIOSEO.php, Line 293
AIOSEO\Plugin\AIOSEO::load @ /wp-content/plugins/all-in-one-seo-pack/app/AIOSEO.php, Line 164
AIOSEO\Plugin\AIOSEO::init @ /wp-content/plugins/all-in-one-seo-pack/app/AIOSEO.php, Line 146
AIOSEO\Plugin\AIOSEO::instance @ /wp-content/plugins/all-in-one-seo-pack/app/AIOSEO.php, Line 381
::aioseo @ /wp-content/plugins/all-in-one-seo-pack/all_in_one_seo_pack.php, Line 87
::include_once @ /wp-settings.php, Line 409
::require_once @ /wp-config.php, Line 78
::require_once @ /wp-load.php, Line 50
::require_once @ /wp-blog-header.php, Line 13
::require @ /index.php, Line 17

Even worse, in Helpers::getUserRoles(), it sets the instance to the global $wp_roles variable.

	public function getUserRoles() {
		global $wp_roles;
		if ( ! isset( $wp_roles ) ) {
			$wp_roles = new \WP_Roles();
		}
		$roleNames = $wp_roles->get_names();
		asort( $roleNames );

		return $roleNames;
	}

The problem is that this happens too early. At this moment, plugins are not all loaded yet, especially because "AIOSEO" starts with 'A' and WP loads plugins in alphabet order. Many user management plugins, e.g., Unlimited Members, are not loaded at all at this moment. Those user management plugins need to hook to role-related hooks to inject their own roles. Because they are not loaded at this moment, the WP_Roles object AIOSEO creates on the stack above will not contain those custom roles, then any other plugins that use WP_User during loading will get wrong roles attached to their WP_User instances.

This causes downstream plugins to behave incorrectly. Some plugins, (e.g. WPForm) call $current_user related functions, which initializes global $current_user with wrong roles. That global variable is cached and is used later on. Thus the corrupted roles info by AIOSEO will be cached forever and will break all custom role related functionalities.

The principle is: A plugin should NEVER initialize global wp_roles or call user related functions during plugin loading. If you look into wp_settings.php, you can see WP itself initializes $wp_roles only AFTER ALL plugins are loaded.

[arnaudbroes]

@congkun-ck thank you for reporting this. I agree that this isn't the best way to do things and I've already developed a fix for this.

The reason why this is currently isn't causing too many issues I think is because most WordPress sites store roles in the database and therefore they would already be accessible when we create that instance of the WP_Roles class. But it is possible to prevent the WP_Roles class from pulling data from the database and instead only use what's dynamically registered.

In any case, I wanted to check if you would like a beta build for this? If so, I'll generate one and post it here down below.

[congkun-ck]

@arnaudbroes thanks for the quick response, Arnaud.
I made a quick mitigation on my website with a small plugin which just resets $current_user on setup_theme, so I am now not blocked. However I am happy to help try the beta fix.

[arnaudbroes]

@congkun-ck I've made some other changes but to fix this I basically just assigned our instance to a variable instead of the global like this -

	public function getUserRoles() {
		global $wp_roles;

		$wpRoles = $wp_roles;
		if ( ! is_object( $wpRoles ) ) {
			// Don't assign this to the global because otherwise WordPress won't override it.
			$wpRoles = new \WP_Roles();
		}

		$roleNames = $wpRoles->get_names();
		asort( $roleNames );

		return $roleNames;
	}

[arnaudbroes]

@congkun-ck we fixed this some months ago in version 4.1.5 so I'll close the issue now.