Settings: Warn when open registration and new user default is privileged

[kraftbj]

Much like our Strong Passwords work, we can help inform site administrators when their actions may be sub-optimal.

WordPress allows a site owner to open registration AND set the default new user level to "Administrator". While this combination may make sense for some sites (on an intranet?), this is typically a really really bad idea.

We should provide some type of confirmation to ensure site administrators are intending to open their site up.

If registration is open and default level is Subscriber (read cap only), the current behavior is fine. If registration is open and other capabilities are included in the default role, we should have some type of checkbox or "are you sure" notice. "By allowing open registration and a default role of {role}, anyone who can visit the site would have the ability to {have full control of your site|publish content|etc}."

We saw this in the wild on a site in support today :).

[subrataemfluence]

Good catch!! Rather having a checkbox to "confirm", I would rather suggest to remove the "Administrator" option from the dropdown from this page. Since admin always has the ability to change a user's role he can easily do so by going to that user's profile any time.

I agree that opening Administrator privilege for a new user registration should not be an option anyway!

[subrataemfluence]

I have added an additional optional parameter which will allow to pass an array of roles which need to be excluded from the dropdown.

Example:

<?php
wp_dropdown_roles( get_option('default_role'), array( 'administrator', 'editor' ) );

Above code won't render Administrator and Editor roles in the dropdown.

However, if called like

<?php
wp_dropdown_roles( get_option('default_role') );

All roles will be available in the dropdown like now.

[kraftbj]

Thanks for the patch @subrataemfluence! A couple of logistical notes on the patch - It is helpful to run the svn diff from the root so you have the full path (e.g. src/wp-admin/includes/template.php instead of just template.php). That'll make it easier to apply to patch.

Also, be sure your code follow WordPress Coding Standards ( ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/ ). There were a few spacing issues that I noticed.

Submitting a follow-up patch that includes updated inline docs, coding standards compliance, and implements the modified wp_dropdown_roles to resolve the ticket issue.

[kraftbj]

A couple of things:

1. We should add a filter so plugins can add additional roles (e.g. Keymaster) to exclude or to allow more roles (e.g. a private intranet where one _does_ want to allow new users as Admins).

2. Does it make sense to exclude roles or exclude certain capabilities?

[subrataemfluence]

Thank you for further fine tuning my proposed patch and thumb up for the introduction of the additional filter!

By the way, I forgot to upload the patch for template.php! :P

Replying to kraftbj:

A couple of things:

1. We should add a filter so plugins can add additional roles (e.g. Keymaster) to exclude or to allow more roles (e.g. a private intranet where one _does_ want to allow new users as Admins).

2. Does it make sense to exclude roles or exclude certain capabilities?

[subrataemfluence]

I would rather suggest this ticket to be labeled as "Feature request" since the current functionality does not have any bug in it.

[roytanck]

We've seen a couple of plugin vulnerabilities recently that allowed attackers to set these options, even while unauthenticated.

The obvious attack vector was to enable registration and set the default role to admin. This was not done through the admin settings page, but through manipulated URLs.

Besides not offering the option in the dropdown, I think core should also not add the user if this combination of settings exists.

Personally, I can think of no use case that would require this combination of settings. It's essentially "please take my site".

[desrosj]

#46744 was marked as a duplicate.

[dd32]

#46744 was closed as a duplicate of this, which I agree with.

The main difference is that this is a warning/only allows selecting safe values in the UI, where #46744 focuses on the malicious setting of options to bad values through a vulnerability that allows setting of options (of which, are common in recent years in plugins).

Preventing a user selecting a dangerous combination is needed, but it also needs to validate that the values in the database are safe to rely upon IMHO
As an example, filter on the default value:

function filter_default_role( $default_role ) {
  // $users_can_register = ....
  if ( $users_can_register && get_role( $default_role )->has_cap( 'manage_options' /* or other cap deemed useful, `publish_posts` could also be used */ ) ) {
    $default_role = 'subscriber'; // Fallback roll for when an unsafe roll has ended up in there
  }
  return $default_role;
}

[desrosj]

#46661 was marked as a duplicate.

[ottok]

I think that both this and #46744 would best be solved by completely preventing the default_role from having the values for 'administrator' and 'editor'. If the database has either of these values, it should just be ignored.

This would categorically fix a whole category of SQL injections that use this trick get admin access to the site. See for example ​https://www.slideshare.net/ottokekalainen/how-to-investigate-and-recover-from-a-security-breach-in-wordpress#29

I am willing to write the patch + unit tests to make sure that if the database has either of these values, it would be ignored, and that in the UI admins can't set the setting to the forbidden values.

I don't see any valid use cases to allow all users to have admin or editor role by default. It is very easy to make a new user with specifically this user role, there is no need to have extra automation to facilitate this and at the same time open a gaping security hole. The trade-off to me is clear: block these dangerous values and let users set user roles in other ways.

Do you agree? Do you want me to write the patch? Would somebody sponsor putting it in then?

[ottok]

Also this should be changed: ​https://wordpress.org/support/article/settings-general-screen/#new-user-default-role

Valid choices are Administrator, Editor, Author, Contributor, or Subscriber.

[jrf]

I've read through this and all related tickets at @ottok's request.

Great input and patches. Thanks everyone who has contributed so far.

Based on everything I've read, I see two distinct points of entry to make changes:

1. The default user role selection drop down on the Options -> General page (display).
2. The update_option() call to update the value for default_role (saving).

For the default user role selection drop down, I would like to suggest the following taking all input given into account:

• Get the default_role from the database.
• If registration is open (users_can_register is enabled), allow the "excluded roles" to be filtered. By default, set this to administrator and editor.
• If registration is open, don't allow administrator as the default role *ever*. The editor role should be allowed, but only when explicitly removed from "excluded roles" via the filter, not as a role available by default.
• If registration is open and the output of the filter would have removed administrator from the "excluded roles", add back administrator and throw a _doing_it_wrong(). This will allow sysadmins to pick up on this being attempted in their error logs.
• Use the output of the "excluded roles" filter in the wp_dropdown_roles() function as proposed in the current patches to limit the roles displayed in the dropdown.
• If the default_role is set to one of the "excluded roles", use subscriber instead. This will also prevent an existing default role of administrator coming from the database from being used.

I agree that using the existing 'editable_roles' filter which filters the roles which will be displayed via the wp_dropdown_roles() function is not strong enough protection as a secondary filter running after "our" filter could undo the removal of administrator. The current patches already take this into account.

For the saving of the option part, we could add a filter hooked into pre_update_option_default_role to check if the value is administrator and if so and only if registration is open, revert it back to subscriber.

I would also like to see some unit tests added for each of these situations to safeguard this change from accidentally being reverted in the future, think tests along the lines of:

• Registration open and default role in database is administrator => actual default role is set to subscriber.
• Registration open and default role in database is something else => actual default role is the same.
• Registration closed and default role in database is administrator => actual default role is set to administator.
• Registration closed and default role in database is something else => actual default role is the same.
• Registration open and default role in database is subscriber, filter on the option changes it to administrator => actual default role is set to subscriber.
• Registration open and default role in database is subscriber, filter on the option changes it to editor => actual default role is set to editor.
• Registration closed and default role in database is subscriber, filter on the option changes it to administrator => actual default role is set to administrator.
• Registration closed and default role in database is subscriber, filter on the option changes it to editor => actual default role is set to editor.

... etc ...

This all could still be bypassed by unhooking the save filter and/or using actions before and after the dropdown display to change registration from open to closed and back again, but for that a malicious plugin would already need to be installed. As far as I c currently see, the above proposed process flow couldn't be bypassed just by manipulating URLs.

Another thing to consider, but this will need further discussion: what about adding a check in the upgrade routine for WP 5.4 to verify the default_role and if 1) registration is open and 2) the default role is set to administrator change it subscriber ?
This will break expectations for site owners which have set the default role to administrator on purpose (intranet), but would - in one go - make all sites where a hack has taken place which has changed this value without the site owner being aware, secure again.

Either way, I hope this feedback helps.

Do you agree? Do you want me to write the patch? Would somebody sponsor putting it in then?

@ottok I'd love to see a patch implementing this and will definitely support such a patch to go in.

Also this should be changed: ​https://wordpress.org/support/article/settings-general-screen/#new-user-default-role

@ottok Good catch and yes, I agree.

[ottok]

2. The update_option() call to update the value for default_role (saving).

This would not protect against the SQL injections I referred to. I was thinking of making a patch that affects fetching the option from the database, and if the database value is 'administrator', the code would ignore that value and return 'subscriber' instead.

[jrf]

Replying to ottok:

2. The update_option() call to update the value for default_role (saving).

This would not protect against the SQL injections I referred to. I was thinking of making a patch that affects fetching the option from the database, and if the database value is 'administrator', the code would ignore that value and return 'subscriber' instead.

You're completely correct, though it would prevent saving of the invalid value from within the WP framework.

An additional filter on the option_default_role, as you suggest, could help in that regards 👍. Just keep in mind that any filter can be unhooked.

[eatingrules]

I'd like to add another vote here to not allow new user default roles to be Editor or Administrator if "Anyone can register" is enabled.

We had a client this morning discover that all new accounts her site were being created as Administrators... She became aware of it only once a customer pointed out to her that she had been granted Admin access after she purchased. We have no idea when/how/why the default setting changed to Administrator (thankfully, at this point haven't found any evidence of other malicious behavior).

Thanks!

[SergeyBiryukov]

Looks like this still needs some work per the latest comments, moving to early 5.5 for now.

[davidbaumwald]

@SergeyBiryukov Is this still on your list to handle early in 5.5?

[davidbaumwald]

Removing the early tag per discussion with @SergeyBiryukov

[verygoode]

Checking in on this one. Ideally, we'd prevent any privileged role from being set as default_role.

However, for now, perhaps a site health warning might help. @SergeyBiryukov

[generosus]

Any plans to revive this topic?

I support the following:

1. Issue a Site Health warning when the New User Default Role selected is Administrator.

2. Add a new settings checkbox titled: Allow New User Default Role: Administrator. The checkbox should be located next to (or below) the New User Default Role setting. When the checkbox is checked, the Site Health warning disappears.

Update appreciated. You guys rock!

Thank you!

[stevejburge]

I think it's worth continuing this discussion. This loophole came up again this week in an Elementor vulnerability.
​https://blog.nintechnet.com/high-severity-vulnerability-fixed-in-wordpress-elementor-pro-plugin/

It might not have entirely mitigated this vulnerability, but it can't hurt to remove "Administrator" by default.

[arunu1996]

#60258 was marked as a duplicate.

[arunu1996]

Replying to eatingrules:

I'd like to add another vote here to not allow new user default roles to be Editor or Administrator if "Anyone can register" is enabled.

We had a client this morning discover that all new accounts her site were being created as Administrators... She became aware of it only once a customer pointed out to her that she had been granted Admin access after she purchased. We have no idea when/how/why the default setting changed to Administrator (thankfully, at this point haven't found any evidence of other malicious behavior).

Thanks!

This same thing happened to one of our client last week.
I suggest preventing new user roles from being set as Editor or Administrator when the "Anyone can register" option is enabled.

[benniledl]

Hey!
What about a check for it on the site health page? Or maybe better not only on site health but maybe even an email notification or just a big red dismissable warning on every admin screen (not just site health)

…curity health check for a risky combination of users_can_register and default_role

Remove administrator and editor from default_role selector and add security health check for a risky combination of users_can_register and default_role

Trac ticket: https://core.trac.wordpress.org/ticket/43936

[benniledl]

Site health check

[benniledl]

Improved Site health check warning with a link

[audrasjb]

Thanks for the PR! I added some change requests concerning various docblocks.
Also, I'm wondering if this sentence should be rewritten in a more inclusive way:

The combination of open registration and the default user role is acceptable.

Indeed, a non-standard combination may be acceptable on some specific installation, so this formulation may not be very inclusive.

I'd suggest the following:

The combination of open registration setting and the default user role may lead to security issues.

What do you think?

Thank you for the review @audrasjb
I added your changes to the pr

@audrasjb Does it ready for 6.5?

[swissspidy]

Code-wise the current PR looks OK and it works as expected too.

The only confusing thing is that if you had set the default role to "Administrator" and then change it, you can't change it back to "Administrator" because it's excluded. You'll have to work around this with a plugin or via wp-admin/options.php

@audrasjb I'll defer this one to you as you self-assigned recently.

[benniledl]

Do I need to update the comment blocks to @since 6.6.0 since 6.5 is now in beta1?

[swissspidy]

Yes it‘s definitely too late for 6.5 now.

I would like to get some more thoughts about my UX remark above for the next iteration of this.
Maybe the admin role could stay in the list but requiring an extra confirm? Like for weak passwords. Just thinking out loud here.
We could also just add the filter but not remove the role by default, deferring that to plugins.
Either way, the Site Health info makes sense to keep.

[benniledl]

In my opinion, it would make sense to remove it per default, and to make this setting accessible only through code or a plugin.
This way, only users who genuinely know what they are doing can enable this setting.
I believe that someone switching from an default admin role to another role and then back to admin may not fully understand what they are doing and the potential risks associated with this setting anyway.

[Mte90]

Wondering if it is related to https://core.trac.wordpress.org/ticket/45359

AS we are talking about email notification for a new user and that ticket is for email notification on password changes.

[swissspidy]

@Mte90 This ticket is not about new email notifications at all, so it's not related.

This ticket is about adding Site Health warnings if user registration is enabled and defaults to an admin role, which is a security risk.

[zodiac1978]

Replying to swissspidy:

This ticket is about adding Site Health warnings if user registration is enabled and defaults to an admin role, which is a security risk.

I hope this ticket is a bit more than just the site health warning. ;) Looking at the patch it does also exclude via filter the administrator and editor role from the dropdown.

This is better than nothing, but I am wondering why some things are not considered:

From @dd32

Preventing a user selecting a dangerous combination is needed, but it also needs to validate that the values in the database are safe to rely upon IMHO

From @ottok

I think that both this and #46744 would best be solved by completely preventing the default_role from having the values for 'administrator' and 'editor'. If the database has either of these values, it should just be ignored.

Also this should be changed: ​https://wordpress.org/support/article/settings-general-screen/#new-user-default-role

Added workflow needs-user-docs for it

From @jrf

2. The update_option() call to update the value for default_role (saving).

If registration is open, don't allow administrator as the default role *ever*. The editor role should be allowed, but only when explicitly removed from "excluded roles" via the filter, not as a role available by default.
If registration is open and the output of the filter would have removed administrator from the "excluded roles", add back administrator and throw a _doing_it_wrong(). This will allow sysadmins to pick up on this being attempted in their error logs.
If the default_role is set to one of the "excluded roles", use subscriber instead. This will also prevent an existing default role of administrator coming from the database from being used.

Additionally, I think #60258 was closed too early. As it has an interesting approach: Having a constant like DISALLOW_FILE_MODS or DISALLOW_FILE_EDIT to disable these two things ("Anyone can register" and the according role select). For most of my installations (besides some membership sites) I would just use this constant in my wp-config.php and the whole feature is grayed out and disabled.

I would love to see these last items discussed and maybe considered for this patch in 6.6
Thanks!

[benniledl]

If this setup is out there in the wild unknowingly to the admin then the site most likely would have been taken over by a malicious actor a long time ago anyway and we should not bother to alter the behavior of the sites since this is most likely on purpose and will annoy anyone who purposefully set their site up like this.

But to warn anyone who is not aware of this, instead of altering the behavior for any existing sites with a dangerous setup of open registration and default user we could send a one time email in the upgrade script that will inform them about their configuration.

We should not prevent advanced users from setting this combination of open registrations and default users but we should make it hard for beginners to accidentally set this up (remove it from the dropdown in the options page)

---

Preventing a user selecting a dangerous combination is needed, but it also needs to validate that the values in the database are safe to rely upon IMHO

I think the site health check and a warning email from the upgrade script will be enough for this.

I think that both this and #46744 would best be solved by completely preventing the default_role from having the values for 'administrator' and 'editor'. If the database has either of these values, it should just be ignored.

I don't think that ignoring the setting made by the user is good in any way, the user should be able to control what the software does. As said, I think most sites with this configuration are purposefully set up like this and just ignoring their settings will annoy them and disrupt the site's operation.

If registration is open, don't allow administrator as the default role *ever*. The editor role should be allowed, but only when explicitly removed from "excluded roles" via the filter, not as a role available by default.

If a user uses filters to make the roles available in the options page then he either knows well enough what he is doing or has at least thought about this setup enough to know what it does and he should be allowed to do it, users should always be able to control their site (with filters as least, we should make it hard to set this up for beginners tho).

If registration is open and the output of the filter would have removed administrator from the "excluded roles", add back administrator and throw a _doing_it_wrong(). This will allow sysadmins to pick up on this being attempted in their error logs.

This will annoy anyone who purposefully set their site up like this, the email and the site health check will be enough.

If the default_role is set to one of the "excluded roles", use subscriber instead. This will also prevent an existing default role of administrator coming from the database from being used.

Again will annoy anyone who purposefully set their site up like this. email and health check should be enough.

Having a constant like DISALLOW_FILE_MODS or DISALLOW_FILE_EDIT to disable these two things ("Anyone can register" and the according role select).

That's a nice thing to have

[ottok]

Thanks @zodiac1978 for quoting me and others recommending to simply disable WordPress from having 'administrator' as the default role under any circumstances.

This "feature" is only being used by bad actors. For the past 6 years I have heard about exactly zero cases where it would make any sense at all to have new users register as administrators by default, but many cases when attackers used this.

There are so many user friendly ways to grant users the administrator role - there is no need to keep this "hole" open for the theoretical case that some site somewhere once would have a legit use case.

I have spoken at many WordCamps about this issue and shown real-world cases when attackers specifically use this vulnerability/configuration error to hack sites. See for example ​https://wordpress.tv/2019/06/10/otto-keka%cc%88la%cc%88inen-how-to-investigate-and-recover-from-a-security-breach-real-life-experiences-with-wordpress/ (explanation about this setting is 20 minutes in the presentation).

This security vulnerability has been in WordPress for decades, and this bug report alone has been open and bike-shedded for 6 years.

I recommend escalating this to the leadership of WordPress (@matt/@nbachiyski/@aaroncampbell?) and get closure on it ASAP to stop more sites from being breached due to this.

[benniledl]

Can we commit the current changes? We can add any additional patches once the discussion is complete. This way, if the ticket is overlooked again, at least the current changes (health check and removal of admin/editor from the settings page) will be included in the next version. @mukesh27 @swissspidy @audrasjb

[ottok]

Note that neither health check and removal of admin/editor from the settings page will help to prevent the security breaches through SQL injections described in https://core.trac.wordpress.org/ticket/43936#comment:12 and earlier comments.

[audrasjb]

Replying to swissspidy:

The only confusing thing is that if you had set the default role to "Administrator" and then change it, you can't change it back to "Administrator" because it's excluded. You'll have to work around this with a plugin or via wp-admin/options.php

As said above by multiple commenters, I don't think it is much of an issue, since installations where Administrator is the default role are very rare use cases. I think we can go with the implementation proposed in PR5893.

[benniledl]

Replying to @audrasjb:

I think we can go with the implementation proposed in PR5893.

I think so too, I updated the docblocks and code comments in PR5893, ready to commit.

[oglekler]

@audrasjb I assume patch can be tested. It looks simple enough to test without instructions, but I just want to check that there are no special cases for multisites.

[pooja1210]

Hi,

Test Report
Patch - ​https://github.com/WordPress/wordpress-develop/pull/5893

Environment:
WordPress: 6.6-alpha
OS: Mac
Browser: Chrome

Verified the Patch: The administrator and editor roles are no longer visible as default roles for new users, and the check appears in the site health report. However, the option to change user role settings is not visible in the site health report. It seems not to be working as expected.

Screenshot:
New User Default Role - ​https://prnt.sc/VolVtX_3AnZ8
Site health report - ​https://prnt.sc/Z3jYGzQufCzR

Thanks

[oglekler]

Thank you @pooja1210 for the report.
It looks like the patch needs a bit more work, I am moving this ticket to 6.7

[benniledl]

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/5893

Environment

• WordPress: 6.6-beta4-20240625.115101
• PHP: 8.1.10
• Server: Apache/2.4.54 (Win64) OpenSSL/1.1.1p PHP/8.1.10
• Database: mysqli (Server: 10.4.25-MariaDB / Client: mysqlnd 8.1.10)
• Browser: Firefox 127.0
• OS: Windows 10/11
• Theme: Twenty Twenty-Four 1.1
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.1.0

Actual Results

1. ✅ Administrator was not an option to select in the options-general page
2. ✅ Site health check displayed correctly when insecure configuration was found

[benniledl]

Site health check

[benniledl]

Options general

[roytanck]

I've been playing around with adding an additional check to wp_insert_user. If we can reliably determine in that function that the user being inserted is self-registering, we could simply return a WP_Error when the default role is privileged, similar to how other checks handle errors.

if ( ! $update && $is_registering ) {
   $user_role = get_role( get_option( 'default_role' ) );
   if ( $user_role->has_cap( 'manage_options' ) ) {
       return new WP_Error( 'empty_data', __( 'Unable to create user. Unsafe default role detected.' ) );
   }
}

Like with the other failed checks, this would show a generic message to the user who's registering ("Unable to register, please contact the site administrator."). Obviously, any mechanism to allow a privileged default role could be added to this, although I still fail to see any plausible use-case.

I'd really like to add something like this in addition to the dropdown filter and the health check to harden against exploits where options are overwritten through other means than the UI.

[benniledl]

can we commit the current changes for 6.7?
@audrasjb