WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#14454 closed defect (bug) (fixed)

function map_meta_cap does not use the user ID when checking super admin

Reported by: dlo Owned by:
Milestone: 3.0.1 Priority: normal
Severity: major Version: 3.0
Component: Role/Capability Keywords: has-patch capability check super admin
Focuses: Cc:

Description

The function map_meta_cap in capabilities.php is checking for super admins in various places like:

case 'edit_users':
		// If multisite these caps are allowed only for super admins.
		if ( is_multisite() && !is_super_admin() )
			$caps[] = 'do_not_allow';
		else
			$caps[] = 'edit_users'; // Explicit due to primitive fall through
		break;

or

case 'delete_user':
	case 'delete_users':
		// If multisite these caps are allowed only for super admins.
		if ( is_multisite() && !is_super_admin() )
			$caps[] = 'do_not_allow';
		else
			$caps[] = $cap;
		break;

In both cases, the function is_super_admin is used without any parameter. That leads to check if the currently connected user is a super admin and not the user passed to the function map_meta_cap.

In my opinion, this is a bug and the correct code should be:

case 'edit_users':
		// If multisite these caps are allowed only for super admins.
		if ( is_multisite() && !is_super_admin($user_id) )
			$caps[] = 'do_not_allow';
		else
			$caps[] = 'edit_users'; // Explicit due to primitive fall through
		break;

and

case 'delete_user':
	case 'delete_users':
		// If multisite these caps are allowed only for super admins.
		if ( is_multisite() && !is_super_admin($user_id) )
			$caps[] = 'do_not_allow';
		else
			$caps[] = $cap;
		break;

I am right ?

Attachments (1)

capabilities.diff (1.2 KB) - added by tmoorewp 6 years ago.

Download all attachments as: .zip

Change History (8)

#1 @nacin
6 years ago

  • Milestone changed from Awaiting Review to 3.0.1

#2 @dlo
6 years ago

  • Component changed from General to Role/Capability

#3 @tmoorewp
6 years ago

  • Cc tim@… added
  • Keywords has-patch added

Here's the changes in patch form.

#4 @nacin
6 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [15479]) Ensure we're checking when looking for is_super_admin() in map_meta_cap(). Bump DB version.
fixes #14454 for the 3.0.1 tag.

#5 @nacin
6 years ago

(In [15480]) Ensure we're checking when looking for is_super_admin() in map_meta_cap(). Bump DB version.
fixes #14454 for trunk.

#6 follow-up: @nacin
6 years ago

  • Severity changed from critical to major

Additionally [15479] for the branch.

Sorry tmoorewp about the props. We scrambled to get this into 3.0.1 and I didn't notice you had a patch here.

#7 in reply to: ↑ 6 @tmoorewp
6 years ago

Replying to nacin:

Additionally [15479] for the branch.

Sorry tmoorewp about the props. We scrambled to get this into 3.0.1 and I didn't notice you had a patch here.

No worries!

Tim

Note: See TracTickets for help on using tickets.