WordPress <= 5.8 - Authenticated Persistent XSS (User role name)

[visse]

Hi there,

First of all, I need to mention this (as requested by @ehtis / H1):

When creating the ticket, please mention in it that the security team has evaluated this and asked you to open a public ticket for discussion.

Intro:

In versions of WordPress, including the latest v5.8, it's possible to inject malicious JavaScript code in the name ($display_name, $details['name']) of any user role.

This vulnerability could be used to infect a website with malicious code or to keep a backdoor for future exploitations. Not all security plugins will detect such injections, cause adding or editing any user role is a legitimate process and all data is stored in the DB.

Important to note that the functionality of adding custom roles is available in many plugins and themes, some of which aren't properly protected from CSRF attacks. Given this vulnerability, such attack vectors can be combined to successfully compromise a website.

Impact:

Malicious JavaScript code injections, the ability to combine attack vectors against the targeted system, which can lead to a complete compromise of the resource.

Steps To Reproduce:

1. Use attached PoC plugin (this is the fastest way to reproduce the JS injection) or use this code in any PHP file on your WordPress website:

   add_role( 'hacker', __( 'Hacker<script>alert(`Visse`);</script>' ), array( 'read' => true, 'edit_posts' => true ) );
   

2. Activate the plugin (you can turn it off right away cause we don't need it anymore - our custom user role will be already injected). Our new role will appear in the database like this:

   s:5:"hacker";a:2:{s:4:"name";s:37:"Hacker<script>alert(`Visse`);</script>";s:12:"capabilities";a:2:{s:4:"read";b:1;s:10:"edit_posts";b:1;}}
   

3. After that injected payload will be triggered on many pages inside the dashboard, f.e.: /wp-admin/users.php | /wp-admin/profile.php | /wp-admin/options-general.php etc. In my PoC plugin there will be a simple alert window.

Additional Information:

Another way to add custom user role is by using plugin, f.e. uListing ​v2.0.4.1 (CSRF scenario):

POST /wp-admin/admin-ajax.php HTTP/2
Host: example.com
Cookie: [admin cookies]
User-Agent: Mozilla/5.0
Content-Type: application/x-www-form-urlencoded; charset=UTF-8
X-Requested-With: XMLHttpRequest
Content-Length: 925

action=stm_save_user_roles&roles%5B0%5D%5Bis_delete%5D=0&roles%5B0%5D%5Bname%5D=Visse%3Cscript%3Ealert(%2FVisse%2F)%3B%3C%2Fscript%3E&roles%5B0%5D%5Bslug%5D=visse&roles%5B0%5D%5Bcapabilities%5D%5Bdefault%5D=1&roles%5B0%5D%5Bcapabilities%5D%5Blisting_limit%5D=1553&roles%5B0%5D%5Bcapabilities%5D%5Blisting_moderation%5D=1&roles%5B0%5D%5Bcapabilities%5D%5Bstm_listing_role%5D=1&roles%5B0%5D%5Bcapabilities%5D%5Ballow_delete_listings%5D=0&roles%5B0%5D%5Bcapabilities%5D%5Bcomment%5D=1&roles%5B1%5D%5Bis_delete%5D=0&roles%5B1%5D%5Bname%5D=Hacker%3Cscript%3Ealert(%2FHacker%2F)%3B%3C%2Fscript%3E&roles%5B1%5D%5Bslug%5D=hacker&roles%5B1%5D%5Bcapabilities%5D%5Bdefault%5D=1&roles%5B1%5D%5Bcapabilities%5D%5Blisting_limit%5D=1337&roles%5B1%5D%5Bcapabilities%5D%5Bcomment%5D=1&roles%5B1%5D%5Bcapabilities%5D%5Blisting_moderation%5D=0&roles%5B1%5D%5Bcapabilities%5D%5Bstm_listing_role%5D=1&roles%5B1%5D%5Bcapabilities%5D%5Bis_open%5D=1

Possible solution:

File: /wp-includes/class-wp-roles.php, line 162:
'name' => $display_name, change to 'name' => strip_tags( $display_name ),.

[visse]

PoC Plugin

[visse]

Screenshot #1

[visse]

Screenshot #2

[visse]

​PoC video (YouTube)

[TobiasBg]

It looks like this requires PHP code access in the first place, correct?

If an attacker has that, the site must be considered compromised anyways. Hardending the add_role function as suggested does not help in any way. The attacked could simply modify the value direct in the database, or use other functions that store values that are later displayed somewhere and thus circumvent any input sanitization that is added to these functions.
And even sanitizing everything everywhere (esc_attr() and so on) won't help as e.g. post content can not be protected like that.

Simply said: If an attacker can run arbitrary PHP code on the site, we can't protect against something like this.

I therefore tend to suggest to close this ticket as invalid, or did I missing something in the explanation?

[visse]

Heya @TobiasBg,

It looks like this requires PHP code access in the first place, correct?

Not really. This, of course, is the easiest and fastest way to show what I am talking about, but a similar result can be achieved when interacting with 3rd-party plugins which is working with user roles in any way, as I already mentioned about in this ticket:

Important to note that the functionality of adding custom roles is available in many plugins and themes, some of which aren't properly protected from CSRF attacks. Given this vulnerability, such attack vectors can be combined to successfully compromise a website.

If an attacker has that, the site must be considered compromised anyways.

True, but:

This vulnerability could be used to infect a website with malicious code or to keep a backdoor for future exploitations.

I mean exactly this part:

to keep a backdoor for future exploitations

If we consider the scenario of an attack against a website through a 3rd-party plugin with a CSRF vulnerability, then this will be a completely different situation with bad consequences. For example of such scenario I mentioned the uListing plugin in this ticket.

And even sanitizing everything everywhere (esc_attr() and so on) won't help as e.g. post content can not be protected like that.

Yep, but you can't really "hide" payloads in any post or page, basically because such actions are predictable and expected, and not everyone will think about checking user roles for some kind of malicious code and so on.

[TobiasBg]

Thanks for the further explanations!

If this can be exploited in other plugins (without needing PHP code access on a site), that is a vulnerability in those plugins and should be reported to the plugin developers and the WordPress Plugins team via email (which probably happened for the uListing plugin that you mentioned, as I can see that it received security fixes in its latest releases).

I guess it can't hurt to add some hardening in WordPress Core though. As the User Role name should never contain HTML code, output escaping (via esc_html() for example) in all places where the role name is printed is probably the best option here. Not only would it counter all possible ways of how the malicious HTML could be added to the database, it would also help uncover that such code exists. So essentially, even though the user role name is coming from the database, it would be considered as "untrusted".

Most likely there are more APIs where data is added to the database via PHP calls and later printed somewhere, so this might have to be part of a broader investigation.

[peterwilsoncc]

Replying to TobiasBg:

I guess it can't hurt to add some hardening in WordPress Core though. As the User Role name should never contain HTML code, output escaping (via esc_html() for example) in all places where the role name is printed is probably the best option here. Not only would it counter all possible ways of how the malicious HTML could be added to the database, it would also help uncover that such code exists. So essentially, even though the user role name is coming from the database, it would be considered as "untrusted".

This is what the security team was considering. Where it's possible to protect against developer mistakes, it is good to do so.

Your earlier comment is correct that it requires PHP so if a developer wishes to act maliciously they can. This is simply to protect against developers being absent minded rather than traditional malware.

[audrasjb]

Hello, thanks for reporting this ticket. It appears this issue wasn't introduced in WP 5.9, so I'm removing the trunk version from the ticket.

[ramon fincken]

Patched, I went with wp_kses and tested it with the POC plugin. Confirmed to work.

[ramon fincken]

patch 53973.1

[ramon fincken]

@desrosj may we continue with this one?

[desrosj]

@ramon-fincken Thanks for the patch!

Some questions and requests:

• Could you open a pull request against wordpress-develop on GitHub so that we can confirm all tests pass as expected?
• Could you also add a few test assertions that demonstrate the problem is gone? This will help protect against accidentally reintroducing the problem.
• I'm curious the reasoning behind returning instead of using the value run through sanitization. For example, running the $slug through sanitize_title() and the $display_name through esc_html() should be enough to then continue on with the function instead of silently failing.

[ramon fincken]

I will get this done, about your 3rd question: they must all validate, so not even via a function as sanitize_title, as the requester will probably use the same name in their code later on.

@ticket 53973, XSS sanitize of role (while adding)

[ramon fincken]

alright I think this is all done from my side @desrosj :)