-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Fix problem with encoding of css entities when post with existing block level custom css is edited by user without unfiltered_html #11104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
292f17d
51f3c12
35eab15
a007350
19537ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2075,8 +2075,31 @@ function _filter_block_content_callback( $matches ) { | |
| * @return array The filtered and sanitized block object result. | ||
| */ | ||
| function filter_block_kses( $block, $allowed_html, $allowed_protocols = array() ) { | ||
| /* | ||
| * Extract per-block custom CSS before KSES processes the attributes. | ||
| * | ||
| * Custom CSS (attrs.style.css) may contain characters like & and > that | ||
| * are valid CSS selectors but would be entity-encoded or stripped by | ||
| * wp_kses(), which treats all values as HTML. Instead, the CSS is | ||
| * temporarily removed, KSES runs on the remaining attributes, and then | ||
| * the CSS is sanitized with CSS-appropriate methods and reinserted. | ||
| */ | ||
| $custom_css = null; | ||
| if ( isset( $block['attrs']['style']['css'] ) ) { | ||
| $custom_css = $block['attrs']['style']['css']; | ||
| unset( $block['attrs']['style']['css'] ); | ||
| } | ||
|
|
||
| $block['attrs'] = filter_block_kses_value( $block['attrs'], $allowed_html, $allowed_protocols, $block ); | ||
|
|
||
| // Sanitize and reinsert the custom CSS using CSS-appropriate methods. | ||
| if ( null !== $custom_css ) { | ||
| $sanitized_css = wp_sanitize_block_custom_css( $custom_css ); | ||
| if ( '' !== $sanitized_css ) { | ||
| $block['attrs']['style']['css'] = $sanitized_css; | ||
| } | ||
| } | ||
|
|
||
| if ( is_array( $block['innerBlocks'] ) ) { | ||
| foreach ( $block['innerBlocks'] as $i => $inner_block ) { | ||
| $block['innerBlocks'][ $i ] = filter_block_kses( $inner_block, $allowed_html, $allowed_protocols ); | ||
|
|
@@ -2124,6 +2147,40 @@ function filter_block_kses_value( $value, $allowed_html, $allowed_protocols = ar | |
| return $value; | ||
| } | ||
|
|
||
| /** | ||
| * Sanitizes per-block custom CSS using CSS-appropriate methods. | ||
| * | ||
| * This function is used instead of wp_kses() for block custom CSS values | ||
| * (attrs.style.css) because wp_kses() treats its input as HTML and would | ||
| * entity-encode valid CSS characters like & (nesting selector) and > | ||
| * (child combinator), or strip content that resembles HTML tags. | ||
| * | ||
| * The sanitization approach mirrors what is used for global styles custom CSS: | ||
| * 1. Strip any HTML tags from the CSS string. | ||
| * 2. Validate that the CSS cannot break out of a `<style>` element. | ||
| * | ||
| * @since 7.0.0 | ||
| * | ||
| * @param string $css Per-block custom CSS string to sanitize. | ||
| * @return string Sanitized CSS string, or empty string if invalid. | ||
| */ | ||
| function wp_sanitize_block_custom_css( $css ) { | ||
| if ( ! is_string( $css ) ) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Strip HTML tags — valid CSS never contains them. | ||
| $css = wp_strip_all_tags( $css ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might have missed this but I think there were doubts about using this for CSS
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the changes are just to push the discussion forward at this stage - if someone has ideas for an alternative, they are welcome change this, but it seems like we should be taking steps to remove anything that is obviously not CSS from those strings - but I do not have an opinion on this. |
||
|
|
||
| // Validate that the CSS cannot break out of a <style> element. | ||
| $validity = wp_validate_css_for_style_element( $css ); | ||
| if ( is_wp_error( $validity ) ) { | ||
| return ''; | ||
| } | ||
|
|
||
| return $css; | ||
| } | ||
|
|
||
| /** | ||
| * Sanitizes the value of the Template Part block's `tagName` attribute. | ||
| * | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting up a fix!
I was wondering: what is the important sanitization step for block CSS attributes?
wp_kses()treats the CSS string as HTML. But should it?I'm wondering if an alternative solution would be to target the
cssattribute and run it throughwp_strip_all_tagsrather thanwp_kses.Or running through something similar (or reuse this same validation in a helper) that @sirreal and @dmsnell worked on in
WP_REST_Global_Styles_Controller::validate_custom_css()for https://core.trac.wordpress.org/ticket/64418There, for users without
unfiltered_html,&and>in block custom CSS were being double-encoded by KSES + JSON, so the CSS broke.(Sorry Jon and Dennis - you've become my default go-to brains trust for this stuff 😄 )