Strip HTML from filter-supplied editorial metadata CSS#945
Merged
Strip HTML from filter-supplied editorial metadata CSS#945
Conversation
The admin list-table CSS for editorial metadata columns is assembled
into a $css_rules array, passed through an
ef_editorial_metadata_manage_posts_css_rules filter, then echoed
inside a <style> block with no escaping. The default selector and
declarations are safe, but a filter callback from another plugin or
theme could smuggle a literal </style> sequence into the keys or
values and break out of the style block into arbitrary HTML.
Each property and declaration is now passed through
wp_strip_all_tags() before being echoed. That removes any HTML tag
the filter may have introduced (including the </style> breakout)
whilst leaving valid CSS syntax untouched — esc_html() would encode
quotes and ampersands, which would corrupt legitimate rules such as
url("…") declarations a plugin might legitimately add.
Replace the manual echo of a <style> block with wp_add_inline_style, attached to the edit_flow-editorial_metadata-styles handle that is already enqueued on the same gate immediately above. This keeps the output in the style pipeline WordPress manages rather than injecting markup mid-admin_enqueue_scripts, and it also drops the remaining phpcs EscapeOutput suppression. The wp_strip_all_tags defences on the filter-supplied selectors and rules are retained: wp_add_inline_style does not escape the string it is handed, so an injected closing </style> tag would still need to be stripped to prevent breakout.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Editorial Metadata builds a small CSS block to give each of its manage-posts columns a sensible minimum width. The rules are assembled into a
\$css_rulesarray, passed through theef_editorial_metadata_manage_posts_css_rulesfilter, then echoed inside a<style>block with no escaping. For Edit Flow's own defaults the output is safe, but because the filter allows arbitrary modification there's nothing stopping a misbehaving plugin or theme from slipping a literal</style>sequence into a selector or declaration — and that would break out of the style block and let whatever followed it be parsed as HTML in the admin.This PR runs both the selector and each declaration through
wp_strip_all_tags()before echoing them. That removes any HTML tag a filter callback may have introduced, including the</style>breakout, whilst leaving valid CSS syntax intact.esc_html()would achieve the same breakout protection but would also encode quotes and ampersands, corrupting any legitimateurl("…")declarations a plugin might have added via the same filter.Test plan
ef_editorial_metadata_manage_posts_css_rulesthat returns a rule containing</style><script>alert(1)</script>; confirm the rendered style block contains neither the</style>nor the script tag