Script tags inside unclosed HTML comment in value passed to WP_Script->localize() breaks page

[rmarscher]

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.

[jonsurrell]

The issue here is very similar to the example I shared in #63851. This is a specific case of that general problem of incorrectly escaped JSON in script tags.

This was fixed by the work on that ticket in [60681].

[sabernhardt]

I cannot set the milestone to 6.9, so I marked this as a duplicate of #63851.