Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#53869 new defect (bug)

Post type / Taxonomy Label Hardening: Prevent Raw HTML tags in output / Media Library eval of HTML entities in label

Reported by: sc0ttkclark's profile sc0ttkclark Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.8
Component: Security Keywords: has-patch
Focuses: Cc:

Description

NOTE: The security team has requested that this ticket be filed, @ehtis is quoted below --

The security team has discussed this internally. We feel that this can be hardened, and we'd prefer it done/discussed publicly in Core Trac.

Overview

It's possible to send unsavory HTML in labels when calling register_post_type(). Whether or not that is intentional and considered a good idea on its own -- it's also possible to send unsavory HTML entities which will then get evaluated by the WordPress media library.

I think Core needs to lock this area down further or make a bigger effort in educating plugin/theme developers that register_post_type() and potentially register_taxonomy() will exhibit this behavior. I did not test register_taxonomy() in this POC but it has similar problems.

Steps To Reproduce

See attached GIF for walkthrough of the JS running from the included POC plugin.

  1. Pass unsavory raw HTML or HTML entities with script tags into various register_post_type() labels (ZIP of POC plugin included)
  2. Navigate to add/edit post screen and click into Featured Image modal (media library tab) to see entities evaluate as actual HTML (the included script tag will run)
  3. Navigate to all posts screen and see that list table outputs HTML unescaped (the included script tag will run)

Recommendations: Core solution

Fix label output vulnerable to Raw HTML

Address the HTML output escaping in the various places these labels get used including but not limited to:

  • all_items (admin menus)
  • name_admin_bar (toolbar)
  • uploaded_to_this_item (passed directly into Media Modal for featured image in add/edit post)
  • filter_items_list (list table)
  • search_items (list table)

Fix label output vulnerable to HTML entities

Address the JS media modal usage of raw HTML sent to DOM:

  • uploaded_to_this_item (passed directly into Media Modal for featured image in add/edit post)

Recommendations: Mitigation solutions for plugin/theme developers

There are a few mitigations options when passing untrusted labels into register_post_type() from CPT plugins or themes that offer CPT building.

These were just a few of the ways I tested to mitigate the problem -- these were unsuccessful as they will leave/create unsavory HTML entities:

  1. wp_strip_all_tags() removes the unsavory HTML and contents of script tags -- Leaves unsavory HTML entities though
  2. strip_tags() but it leaves script contents (it also leave
  3. wp_kses_post() removes the unsavory HTML (but leaves unsavory entities)
  4. htmlspecialchars() will convert unsavory HTML and introduce HTML entities which will get evaluated by the uploaded_to_this_item label usage.

The recommended solution that works for BOTH raw HTML and HTML entities, including double entity attacks (encoding twice and passing it in) on the media library uploaded_to_this_item label:

htmlspecialchars( wp_strip_all_tags( htmlspecialchars_decode( $label ) ) ) will strip potential HTML tags from entities while also removing the contents of script/style tags. The final hardening by htmlspecialchars() will ensure nothing else makes it to the media library uploaded_to_this_item label output.

Impact

This allows the attacker to insert scripts into normal code that would get passed into register_post_type() including core WP hooks. It can most commonly be used through CPT builders although many free ones have mitigation steps which they each much do on their side to prevent these kinds of attacks.

The largest impact I believe is in the uploaded_to_this_item label which has raw HTML or HTML entities evaluated directly into the DOM which I believe to be unintentional.

At minimal -- a solution to this ticket should deal with the uploaded_to_this_item HTML entities problem. I'd love to see HTML disallowed across the other labels as we have many other better solutions with JS that can plugins can work with instead.

Attachments (1)

xss-cpt-label-test.zip (3.8 KB) - added by sc0ttkclark 3 years ago.
POC plugin to test with (includes HTML, HTML entity, and double HTML entity tests

Download all attachments as: .zip

Change History (4)

@sc0ttkclark
3 years ago

POC plugin to test with (includes HTML, HTML entity, and double HTML entity tests

This ticket was mentioned in PR #1544 on WordPress/wordpress-develop by sc0ttkclark.


3 years ago
#2

  • Keywords has-patch added

This PR addresses issues where .html() will insert HTML entities into the 'Filter media' <option> tags -- which the entities will then get rendered as real HTML tags by the JS so script tags / onload etc will run.

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

#3 @sc0ttkclark
3 years ago

I've submitted a potential PR showing what a patch might be for this at the JS level for uploaded_to_this_item (via l10n.uploadedToThisPost) but more testing will be needed to ensure it's OK for i18n/l10n values coming through.

Note: See TracTickets for help on using tickets.