Make WordPress Core

Opened 4 years ago

Closed 9 months ago

Last modified 7 months ago

#14578 closed defect (bug) (fixed)

Default User Role isn't checked against defined roles, causing unexpected resets to Administrator

Reported by: Ivolution Owned by: garyc40
Milestone: 3.7 Priority: normal
Severity: major Version: 3.0.1
Component: Role/Capability Keywords: has-patch 3.2-early commit
Focuses: Cc:


Take these steps:

  1. Activate a plugin that creates role on activation. For example, it calls "add_role( 'photo_uploader', 'Photo Uploader', array( 'read') );"
  2. In General Settings, set the Default User Role to this new role, 'Photo Uploader'.
  3. Deactivate the plugin, removing the roles: "remove_role( 'photo_uploader');"
  4. In General Settings, the Default User Role now displays 'Administrator'. (In the database, it still says 'photo_uploader'.)
  5. When creating a new user (as admin), the role dropdown-box now displays 'Administrator' as role for this new user. This new user _will_ have role 'Administrator' if an unsuspecting admin does not explicitly alter the role in the dropdown-box.

This way, an unsuspecting adminstrator might accidentally create new admins for his blog.

I have also tested this for new users registering themselves. Fortunately, they are assigned the role 'None', not 'Administrator'.


Ivo van der Linden
(employee of LaQuSo @ Eindhoven University of Technology)

Attachments (3)

garyc40.14578.diff (4.0 KB) - added by garyc40 4 years ago.
reverse role dropdown list, add a verification filter for 'default_role', update 'default_role' when remove_role()
14578.diff (2.9 KB) - added by wonderboymusic 18 months ago.
14578.2.diff (2.9 KB) - added by wonderboymusic 12 months ago.

Download all attachments as: .zip

Change History (17)

comment:1 scribu4 years ago

  • Component changed from General to Role/Capability

comment:2 Denis-de-Bernardy4 years ago

  • Cc Denis-de-Bernardy added

comment:3 dd324 years ago

We should probably check on remove_role() to see if it's the default role, and if so, revert back to subscriber in that case (Assuming the role exists)

comment:4 dd324 years ago

  • Keywords needs-patch added; plugin administrator security removed

comment:5 nacin4 years ago

We should probably reverse the results from get_editable_roles() there, so they are listed in ascending order (for the default roles).

Dion's take definitely makes sense. We could also drop a filter in the options API to verify the role's existence that way (as roles aren't always stored in the DB), or just stick an update_option call in options-general right before the role dropdown. Cheap, but effective here and elsewhere.

comment:6 nacin4 years ago

  • Keywords 3.2-early added
  • Milestone changed from Awaiting Review to Future Release
  • Summary changed from Security issue after plugin deactivation (by accidentally creating administrators) to Default User Role isn't checked against defined roles, causing unexpected resets to Administrator

comment:7 garyc404 years ago

  • Owner set to garyc40
  • Status changed from new to assigned

garyc404 years ago

reverse role dropdown list, add a verification filter for 'default_role', update 'default_role' when remove_role()

comment:8 garyc404 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

wonderboymusic18 months ago

comment:9 wonderboymusic18 months ago

  • Keywords needs-testing removed
  • Milestone changed from Future Release to 3.6

Patch refreshed against trunk excludes the PHP4 by-ref nonsense

comment:10 ryan14 months ago

  • Milestone changed from 3.6 to Future Release

wonderboymusic12 months ago

comment:11 wonderboymusic12 months ago

  • Milestone changed from Future Release to 3.7

14578.2.diff is fuzz-less

comment:12 nacin9 months ago

  • Keywords commit added

Hmm, looking at it again, this might be better on save? Agree with the rest, though.

comment:13 nacin9 months ago

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

In 25695:

Reverse the order of roles in wp_dropdown_roles(). Reset to 'subscriber' when the default role is removed and when a save is invalid.

props garyc40, wonderboymusic.
fixes #14578.

comment:14 nofearinc7 months ago

#15636 was marked as a duplicate.

Note: See TracTickets for help on using tickets.