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.

[jonsurrell]

In 61485:

Script Loader: Use HTML API to generate SCRIPT tags.

Script tags have complicated and unintuitive parsing rules that make them difficult to author correctly. The HTML API automatically escapes script tag contents as necessary and will set attributes correctly. Using the HTML API to generate SCRIPT tags improves safety when working with SCRIPT tags, resolving a class of issues that have manifested repeatedly.

Changeset [61418] applied the HTML API to generate style tags in a similar way.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10639.

Props jonsurrell, dmsnell, westonruter.
Fixes #64500. See #64419, #40737, #62797, #63851, #51159.