[wp-trac] [WordPress Trac] #62787: Twenty Nineteen: sanitize output of twentynineteen_custom_colors_css() (was: Twenty Nineteen : PHPCS Fixes)

[WordPress Trac]

#62787: Twenty Nineteen: sanitize output of twentynineteen_custom_colors_css()
---------------------------+-------------------------------
 Reporter:  pitamdey       |       Owner:  (none)
     Type:  defect (bug)   |      Status:  new
 Priority:  normal         |   Milestone:  Awaiting Review
Component:  Bundled Theme  |     Version:
 Severity:  normal         |  Resolution:
 Keywords:  has-patch      |     Focuses:  coding-standards
---------------------------+-------------------------------
Changes (by sabernhardt):

 * focuses:  php-compatibility => coding-standards
 * component:  Themes => Bundled Theme


Old description:

> In theme Twenty Nineteen , I can see one PHPCS Warning in function.php
> file i.e.
> All output should be run through an escaping function (see the Security
> sections in the WordPress Developer Handbooks), found
> 'twentynineteen_custom_colors_css'
>
> Here :
> {{{
> <style type="text/css" id="custom-theme-colors" <?php echo
> is_customize_preview() ? 'data-hue="' . absint( $primary_color ) . '"' :
> ''; ?>>
>                 <?php echo twentynineteen_custom_colors_css(); ?>
>         </style>
> }}}

New description:

 In theme Twenty Nineteen , I can see one PHPCS Warning in `functions.php`
 file i.e.
 All output should be run through an escaping function (see the Security
 sections in the WordPress Developer Handbooks), found
 'twentynineteen_custom_colors_css'

 Here :
 {{{
 <style type="text/css" id="custom-theme-colors" <?php echo
 is_customize_preview() ? 'data-hue="' . absint( $primary_color ) . '"' :
 ''; ?>>
                 <?php echo twentynineteen_custom_colors_css(); ?>
         </style>
 }}}

--

Comment:

 1. The default output of `twentynineteen_custom_colors_css()` is CSS only,
 but that is filterable.
 2. At least [https://wpdirectory.net/search/01JH3BKN4PCYJNR237TRGK0CDD two
 plugins] use the filter.
 3. To remove any added HTML entirely, consider `wp_strip_all_tags()`
 instead of `esc_html()`. Note that either function could affect a tag name
 within a CSS comment (`esc_html` would be better for that unlikely
 possibility).
 4. `twentynineteen_custom_colors_css()` outputs code in two places. If the
 [https://github.com/WordPress/wordpress-
 develop/blob/287b2f8c961e8a67b2a232d37273b6ddfa611c28/src/wp-
 content/themes/twentynineteen/functions.php#L344-L346 'custom-theme-
 colors' styles] escape or remove HTML tags, then the
 [https://github.com/WordPress/wordpress-
 develop/blob/287b2f8c961e8a67b2a232d37273b6ddfa611c28/src/wp-
 content/themes/twentynineteen/functions.php#L321 inline style for
 'twentynineteen-editor-customizer-styles'] probably should use the same
 function.

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