[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:  7.0.1
Component:  Formatting    |     Version:  trunk
 Severity:  normal        |  Resolution:
 Keywords:  needs-patch   |     Focuses:
--------------------------+---------------------

Comment (by dmsnell):

 @nextendweb as unfortunate as it is, I’m glad you found this issue. this
 arose because of the fix you identified in `wp_kses_hair()`, but I don’t
 assess this as a regression. if we go back to the report in the
 description

 {{{#!php
 <?php
 echo wp_kses( '<div style="background-image:
 url("https://localhost/image.jpg");"></div>', $allowed );
 }}}

 this case alone demonstrates the problem in `safecss_filter_attr()`, which
 never handled this valid and common `style` attribute. the root of the
 problem appears to be that `safecss_filter_attr()` assumes it will receive
 decoded HTML values and return decoded HTML values, but `wp_kses_hair()`
 and `wp_kses_attr_check()` are assuming raw and unescaped content.

 the change in 7.0 is that we made `wp_kses_hair()` normalize inputs to
 prevent further breakage downstream, which is a common phenomena in Core.
 to that end, this one particular case will appear as a regression (even
 while many other cases were resolved), but the problem is in the
 interaction of raw and escaped HTML, plus the bigger problem that //we
 can’t split CSS by `;`//. so even if we fix or revert the change to
 `wp_kses_hair()` and bring back the other broken scenarios, we still
 haven’t fixed the bug that broke your code.

 in the meantime, if we want to address this, I think the least harmful
 resolution would be to wrap the call to `safecss_filter_attr()` in
 `wp_kses_attr_check()` so that it escapes and then unescapes the HTML.

 {{{
 diff --git a/src/wp-includes/kses.php b/src/wp-includes/kses.php
 index 062f853085..77c3701a66 100644
 --- a/src/wp-includes/kses.php
 +++ b/src/wp-includes/kses.php
 @@ -1556,7 +1556,8 @@ function wp_kses_attr_check( &$name, &$value,
 &$whole, $vless, $element, $allowe
         }

         if ( 'style' === $name_low ) {
 -               $new_value = safecss_filter_attr( $value );
 +               $decoded_value = WP_HTML_Decoder::decode_attribute( $value
 );
 +               $new_value     = safecss_filter_attr( $decoded_value );

                 if ( empty( $new_value ) ) {
                         $name  = '';
 @@ -1565,7 +1566,7 @@ function wp_kses_attr_check( &$name, &$value,
 &$whole, $vless, $element, $allowe
                         return false;
                 }

 -               $whole = str_replace( $value, $new_value, $whole );
 +               $whole = str_replace( $value, esc_attr( $new_value ),
 $whole );
                 $value = $new_value;
         }

 }}}

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