WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 3 months ago

#53973 new defect (bug)

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

Reported by: visse Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: trunk
Component: Security Keywords: dev-feedback
Focuses: administration Cc:

Description

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 ),.

Attachments (3)

evil-user-role.php (305 bytes) - added by visse 3 months ago.
PoC Plugin
Screenshot 2021-08-21 at 03.55.26.png (264.8 KB) - added by visse 3 months ago.
Screenshot #1
Screenshot 2021-08-21 at 03.57.23.png (654.9 KB) - added by visse 3 months ago.
Screenshot #2

Download all attachments as: .zip

Change History (8)

@visse
3 months ago

PoC Plugin

#2 @TobiasBg
3 months ago

  • Keywords close added; needs-patch removed

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?

#3 @visse
3 months ago

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.

#4 follow-up: @TobiasBg
3 months ago

  • Keywords dev-feedback added; close removed

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.

#5 in reply to: ↑ 4 @peterwilsoncc
3 months ago

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.

Note: See TracTickets for help on using tickets.