[wp-trac] [WordPress Trac] #65270: wp_kses() corrupts valid CSS background-image: url(...) declarations into style=")" 7.0-RC4

[WordPress Trac]

#65270: wp_kses() corrupts valid CSS background-image: url(...) declarations into
style=")" 7.0-RC4
--------------------------+------------------------------
 Reporter:  nextendweb    |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Formatting    |     Version:  trunk
 Severity:  major         |  Resolution:
 Keywords:                |     Focuses:
--------------------------+------------------------------

Comment (by nextendweb):

 **Another example with & which also breaks**
 {{{#!php
 <?php
 echo wp_kses('<div style="background-image:
 url(https://localhost/image.jpg?a=1&b=2);"></div>', [
     'div' => [
         'style' => ['background-image']
     ]
 ]);
 }}}

 **Related Source Code**

 **`wp_kses_hair()`**

 The issue appears to begin in `wp_kses_hair()` when the parsed attribute
 value is reconstructed and syntax characters are entity-encoded:

 {{{
 function wp_kses_hair( $attr, $allowed_protocols ) {
 $attributes = array();
 $uris       = wp_kses_uri_attributes();

 $processor = new WP_HTML_Tag_Processor( "<wp {$attr}>" );
 $processor->next_token();

 $attribute_names = $processor->get_attribute_names_with_prefix( '' );
 if ( null === $attribute_names || 0 === count( $attribute_names ) ) {
         return $attributes;
 }

 $syntax_characters = array(
         '&' => '&',
         '<' => '<',
         '>' => '>',
         "'" => ''',
         '"' => '"',
 );

 foreach ( $attribute_names as $name ) {
         $value   = $processor->get_attribute( $name );
         $is_bool = true === $value;
         if ( is_string( $value ) && in_array( $name, $uris, true ) ) {
                 $value = wp_kses_bad_protocol( $value, $allowed_protocols
 );
         }

         // Reconstruct and normalize the attribute value.
         $recoded = $is_bool ? '' : strtr( $value, $syntax_characters );
         $whole   = $is_bool ? $name : "{$name}=\"{$recoded}\"";

         $attributes[ $name ] = array(
                 'name'  => $name,
                 'value' => $recoded,
                 'whole' => $whole,
                 'vless' => $is_bool ? 'y' : 'n',
         );
 }

 return $attributes;

 }
 }}}

 For this input:

 {{{

 <div style="background-image: url('https://localhost/image.jpg');"></div>
 }}}

 `wp_kses_hair()` returns the style value as:

 {{{
 background-image: url('https://localhost/image.jpg');
 }}}

 `wp_kses_attr_check()`

 That encoded value is then passed into `safecss_filter_attr()`:

 {{{
 if ( 'style' === $name_low ) {
 $new_value = safecss_filter_attr( $value );
 }
 }}}

 Debug output:

 {{{
 string(63) "background-image:
 url('https://localhost/image.jpg');"
 string(1) ")"
 }}}

 `safecss_filter_attr()`

 The malformed result appears to happen when `safecss_filter_attr()` splits
 declarations with `explode( ';', ... )`:

 {{{
 function safecss_filter_attr( $css, $deprecated = '' ) {
 if ( ! empty( $deprecated ) ) {
 _deprecated_argument( **FUNCTION**, '2.8.1' ); // Never implemented.
 }

 ```
 $css = wp_kses_no_null( $css );
 $css = str_replace( array( "\n", "\r", "\t" ), '', $css );

 $allowed_protocols = wp_allowed_protocols();

 $css_array = explode( ';', trim( $css ) );
 ```

 }}}

 Because `'` contains a semicolon, the CSS is split incorrectly:

 {{{
 background-image: url('https://localhost/image.jpg');
 }}}

 becomes roughly:

 {{{
 background-image: url(&apos
 [https://localhost/image.jpg&apos](https://localhost/image.jpg&apos)
 )
 }}}

 The last fragment survives sanitization and produces:

 {{{

 <div style=")"></div>
 }}}

 **Possible Solutions**

 **Solution 1**: Preserve raw quote characters in `wp_kses_hair()`

 Avoid storing the entity-encoded value in the internal `value` field
 returned by `wp_kses_hair()`.

 Potential adjustment:

 {{{
 $attributes[ $name ] = array(
 'name'  => $name,
 'value' => $is_bool ? '' : $value,
 'whole' => $whole,
 'vless' => $is_bool ? 'y' : 'n',
 );
 }}}

 This keeps `value` as:

 {{{
 background-image: url('https://localhost/image.jpg');
 }}}

 while `whole` can remain safely escaped for HTML reconstruction.

 Pros:

 * Keeps CSS sanitization operating on raw CSS values.
 * Avoids mixing HTML entity encoding with CSS parsing.
 * Fixes the WordPress 7.0 regression for valid single-quoted `url(...)`
 values.

 Cons:

 * Does not fix existing failures involving already entity-encoded CSS
 values such as `"`.

 **Solution 2**: Decode HTML entities before CSS declaration parsing

 Decode HTML entities in `safecss_filter_attr()` before splitting CSS
 declarations:

 {{{
 $css = html_entity_decode( $css, ENT_QUOTES | ENT_HTML5,get_option(
 'blog_charset' ) );
 }}}

 before:

 {{{
 $css_array = explode( ';', trim( $css ) );
 }}}

 This converts:

 {{{
 url('https://localhost/image.jpg')
 }}}

 back into:

 {{{
 url('https://localhost/image.jpg')
 }}}

 before `explode( ';', ... )` runs.

 Pros:

 * Fixes the WordPress 7.0 regression.
 * Also fixes pre-existing failures involving entity-encoded CSS values
 such as `"`.
 * Minimal patch with low implementation complexity.

 Cons:

 * Changes parsing input semantics globally within `safecss_filter_attr()`.
 * Decodes all HTML entities before CSS parsing, which could theoretically
 affect obscure edge cases.

 **Solution 3**: Decode HTML entities before CSS declaration parsing and
 then enconde again
 {{{#!php
 <?php
 function safecss_filter_attr( $css, $deprecated = '' ) {

 ...

     $css        = strtr( $css, array(
         '&' => '&',
         '<' => '<',
         '>' => '>',
         ''' => "'",
         '"' => '',
     ) );

     $css_array = explode( ';', trim( $css ) );

 ...

     return strtr( $css, array(
         '&' => '&',
         '<' => '<',
         '>' => '>',
         "'" => ''',
         '"' => '"',
     ) );
 }
 }}}

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/65270#comment:1>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform