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 | 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.
- Pass unsavory raw HTML or HTML entities with script tags into various
register_post_type()
labels (ZIP of POC plugin included) - 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)
- 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:
wp_strip_all_tags()
removes the unsavory HTML and contents of script tags -- Leaves unsavory HTML entities thoughstrip_tags()
but it leaves script contents (it also leavewp_kses_post()
removes the unsavory HTML (but leaves unsavory entities)htmlspecialchars()
will convert unsavory HTML and introduce HTML entities which will get evaluated by theuploaded_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)
Change History (4)
#1
@
3 years ago
GIF screencast is here, it wouldn't upload because of the size: https://p199.p4.n0.cdn.getcloudapp.com/items/Qwu5x498/f24b4988-e0d1-4573-a8f3-3d4e48e425b9.gif
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
POC plugin to test with (includes HTML, HTML entity, and double HTML entity tests