Make WordPress Core

Opened 15 years ago

Closed 15 years ago

Last modified 15 years ago

#8764 closed defect (bug) (fixed)

user-edit.php - use wp_dropdown_roles() rather than duplicating its code (security)

Reported by: jeremyclarke's profile jeremyclarke Owned by: jeremyclarke's profile jeremyclarke
Milestone: 2.8 Priority: normal
Severity: normal Version:
Component: Security Keywords: has-patch needs-testing capabilities
Focuses: Cc:

Description

For history see: #6014

I'm updating that patch so it can be added to 2.8, but i'm splitting up the various parts so they can be added more easily.

Part 1 was #8760, now commited. Part 2 is #8761, which is imortant for this patch.

What I want (same as #8760): To add security to the capabilities system because right now edit_users can't be delegated to non-admins (in our case our content editors). If someone has 'edit_users' they can make themself admin because nothing stops them from editing themselves or others to be admin. I think it should be integrated into core but don't care enough to fight. It can be done with a plugin so my priority is to make sure that my plugin (and Role Manager plugin) can hook into the appropriate places and add a role comparison such that wp only lets people edit users/roles "lower" than them (i.e. users that don't have any powers that the editor don't have).

This particular patch is very simple. All it does is change the user editing page (user-edit.php, the one you use to edit the details of a specific user) to use the function wp_dropdown_roles(), which is already used on the user listing page users.php. The function is general and this is exactly the purpose it was designed for.

In #8761 I added a filter to wp_dropdown_roles() that is needed to secure the edit_user capability by limiting the visible roles in dropdowns like this. To keep the security effects of this filter consistent and effective against malicious users (hackers exploiting accounts or malcontent users) the wp_dropdown_roles function must be used everywhere that roles are set for users.

In terms of the code, I've removed most of the redundant looping logic and replaced it with wp_dropdown_roles. In the old version, it would loop through the capabilities and add selected=selected to any that matched the relevant user's roles. In the new version it instead passes in the primary (selected) role to wp_dropdown_roles as a parameter. The primary role is determined in teh same way as on the user listing page (i.e. this is how the text in the 'role' column of users.php is determined):

$user_roles = $profileuser->roles;
$user_role = array_shift($user_roles);

That logic should be pushed into its own function imho (as stated in the comment) but i wanted to keep this ticket as simple as possible.

For normal setups this patch should have zero noticeable effect. Everything should work like normal but the code is cleaner and more modular.

For those that use my plugin code (hopefully part of Role Manager ASAP, should have been a long time ago), this will allow them to delegate user_edit privileges to multiple people, which is vital to large WP sites. For more infomation on seeing this in action and to test this patch with it, see #8761.

Thanks for your consideration.

Attachments (2)

user-edit_use_wp_dropdown_roles.diff (1.7 KB) - added by jeremyclarke 15 years ago.
use wp_dropdown_roles in user-edit.php
jan6-fix-user-edit-notice.diff (807 bytes) - added by jeremyclarke 15 years ago.
fix the php notice caused by earlier patch.

Download all attachments as: .zip

Change History (4)

@jeremyclarke
15 years ago

use wp_dropdown_roles in user-edit.php

#1 @ryan
15 years ago

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

(In [10319]) use wp_dropdown_roles(). Props jeremyclarke. fixes #8764

@jeremyclarke
15 years ago

fix the php notice caused by earlier patch.

#2 @ryan
15 years ago

(In [10324]) Fix notice. Props jeremyclarke. see #8764

Note: See TracTickets for help on using tickets.