Make WordPress Core

Opened 8 years ago

Last modified 8 years ago

#40737 new defect (bug)

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

Reported by: rmarscher's profile rmarscher Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Script Loader Keywords: has-patch
Focuses: javascript Cc:

Description

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.

Attachments (1)

localize-wp-scripts-split-script-tag-in-output.diff (705 bytes) - added by rmarscher 8 years ago.

Download all attachments as: .zip

Change History (2)

#1 @ocean90
8 years ago

  • Keywords has-patch added
  • Version trunk deleted
Note: See TracTickets for help on using tickets.