[wp-trac] [WordPress Trac] #53869: Post type / Taxonomy Label Hardening: Prevent Raw HTML tags in output / Media Library eval of HTML entities in label

[WordPress Trac]

#53869: Post type / Taxonomy Label Hardening: Prevent Raw HTML tags in output /
Media Library eval of HTML entities in label
--------------------------+-----------------------------
 Reporter:  sc0ttkclark   |      Owner:  (none)
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  Security      |    Version:  5.8
 Severity:  normal        |   Keywords:
  Focuses:                |
--------------------------+-----------------------------
 **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.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/53869>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform