[wp-trac] [WordPress Trac] #40737: Script tags inside unclosed HTML comment in value passed to WP_Script->localize() breaks page

[WordPress Trac]

#40737: Script tags inside unclosed HTML comment in value passed to
WP_Script->localize() breaks page
---------------------------+-----------------------------
 Reporter:  rmarscher      |      Owner:
     Type:  defect (bug)   |     Status:  new
 Priority:  normal         |  Milestone:  Awaiting Review
Component:  Script Loader  |    Version:  trunk
 Severity:  normal         |   Keywords:
  Focuses:  javascript     |
---------------------------+-----------------------------
 Hi. Thanks for your time. I encountered an edge case with the
 WP_Script->localize() method that can cause malformed html and cause
 javascript on the page to break.

 Our site has a custom field for adding pixel tracking code html snippets.
 On the post edit admin page, the values from the custom fields are run
 through the localize method by the Wordpress-SEO plugin which adds their
 values to the wpseoReplaceVarsL10n var.

 One of our users pasted code into this field like the following:

 {{{
 <!-- Code comment --!>
 <script>
 console.log('test');
 </script>
 }}}

 This gets serialized to JSON by WP_Script->localize and ends up being
 output like this. I simplified the structure a bit for the example.

 {{{
 <script type='text/javascript'>
 /* <![CDATA[ */
 var wpseoReplaceVarsL10n = {"custom_header_html":"<!-- Code comment
 --!>\r\n<script>\r\nconsole.log('test');\r\n<\/script>"};
 /* ]]> */
 </script>
 }}}

 This catches some interesting edge behavior in the browser. Notice the
 code comment is not actually closed correctly because it has {{{--!>}}}
 instead of {{{-->}}}. When testing that in html, I noticed that browsers
 seem to treat it as closing the comment and it doesn't break the page. It
 must be a common enough mistake.

 If the field value is changed it have the proper {{{-->}}}, there are no
 issues. It also requires the {{{<script>}}} tag to be inside the unclosed
 comment for it to be an issue.

 It can be worked around by replacing {{{<script>}}} tags in the output
 with a string concatenated {{{<scri" + "pt>}}} version. Other potential
 solutions would be replacing the {{{<}}} and {{{>}}} tag markers with
 unicode points like the {{{JSON_HEX_TAG}}} option to {{{json_encode()}}}
 in PHP 5.3+ does. But that will cause a lot more replacements and also
 increase the page size more.

 I think `str_replace( '<script', '<scr" + "ipt', wp_json_encode( $l10n )
 )` is probably the least intrusive solution. I attached a patch with that
 update for your consideration.

 Thanks for your time reviewing this.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/40737>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform