[wp-trac] Re: [WordPress Trac] #6014: Hook needed on wp_dropdown_roles() to secure 'edit_users' capability (see last comment)

[WordPress Trac]

#6014: Hook needed on wp_dropdown_roles() to secure 'edit_users' capability (see
last comment)
--------------------------+-------------------------------------------------
 Reporter:  jeremyclarke  |        Owner:  pishmishy
     Type:  defect        |       Status:  assigned 
 Priority:  normal        |    Milestone:  2.6      
Component:  Security      |      Version:           
 Severity:  major         |   Resolution:           
 Keywords:  has-patch     |  
--------------------------+-------------------------------------------------
Changes (by jeremyclarke):

  * keywords:  => has-patch
  * summary:  Users given the 'edit_users' capability can alter and create
              new users above their user level. => Hook
              needed on wp_dropdown_roles() to secure
              'edit_users' capability (see last comment)

Comment:

 Okay! I got everything working. It requires edits to 2 files in 3 places
 total.

 1) in wp-admin/includes/template.php
     function user_row() - This function generates the rows of the table
 seen on users.php, it was already checking if the logged-in user had privs
 to edit that row's user. I added an extra variable based on that check
 which adds disabled=true to the checkbox if the row's user is not
 editable. This has no effect on anything unless a plugin hooks into the
 user_has_cap filter (so it won't hurt normal users), but it allows plugin
 authors (i.e. Role Manager) to disable the checkboxes when necessary
 (rather than throwing errors if you try to modify someone you dont' have
 privs to modify using the checkboxes and pulldowns at the bottom).

 2) in wp-admin/includes/template.php
     function wp_dropdown_roles() - This function was generating the role
 lists for the top and bottom of users.php, I modified it to add a filter
 on the role_names (as well as making a copy of it from $wp_roles rather
 than using it directly, so that plugins can unset elements in the copy (to
 hide those list items) without removing them from the actual $wp_roles
 object). I also changed the name of the function's argument to 'selected'
 rather than default. The old name was because on users.php the 'default'
 new role was set as selected by default, but when used on user_edit.php
 the selected option should be the actual role of the user you're editing,
 so the new argument name reflects what the value will actually do.

 3) in wp-admin/user-edit.php
         The main reason for changing this file was to integrade the
 wp_dropdown_roles() function rather than having it print the roles from
 scratch into the <select>. As a result I removed several lines that were
 no longer necessary and reworked a few lines so that they made more sense
 with the wp_dropdown_roles() function. I left several comments and some of
 the old code commented out in case someone wants to check that everything
 makes sense.

 I have tested the changes with an SVN copy of wp2.5 and the newest version
 of the Role Manager plugin. Between the new hook on wp_dropdown_roles()
 and some hooks on the user_has_cap filter I was able to secure the user
 editing interface so that users can only edit other users at or below
 their level (and can't promote users or themselves to any role or
 capability that they do not have).

 Any feedback is appreciated and please let me know if there are problems
 with this. I think this should definitely be part of core (especially
 becaue I avoided actually putting any of the user comparison code into the
 patch, so WP remains pure and hierarchy-less). Thanks in advance to anyone
 who tests this.

 [I am also attatching the sample plugin I wrote for hooking into this and
 adding the security to Role Manager. To see the effect you will have to
 install Role Manager, give the 'edit_users' privilege to a user or role
 that is not administrator, and turn on this extra plugin called "Role
 Manager Security Core Patch Example Plugin". At that point you'll see that
 the lower user is unable to edit administrators or anyone above their
 level, and won't see administrator in role dropdowns on user_edit.php or
 users.php]

-- 
Ticket URL: <http://trac.wordpress.org/ticket/6014#comment:8>
WordPress Trac <http://trac.wordpress.org/>
WordPress blogging software