wp_set_script_translations() accepts and evaluates <script> tag included in JSON

[Takahashi_Fumiki]

NOTE

This ticket is already posted at May 22nd 2020 to hackerone.com #880749 and reviewed by the security team.
I've created this ticket for public hardening under their instruction.

Description:

wp_set_script_translations is a function that provides translation strings for [@wordpress/i18n](​https://developer.wordpress.org/block-editor/packages/packages-i18n/) library. The JSON will be output just before the targeted script without checking the string <script> in JSON file. As a result, a cracker can inject and execute any JavaScript.

Steps To Reproduce:

1. Create a small plugin or theme which uses my-script.js and using domain my-theme.
2. Create my-theme.pot and translate it in ja.po which includes string like </script><script>alert('Hacked!')<script> .
3. Convert ja.po to JSON my-theme-ja-my-script.json via jed ​https://www.npmjs.com/package/jed
4. Execute wp_set_script_translations( 'my-script', 'my-theme', get_template_directory() . '/languages' )
5. Enqueue my-script.js in anywhere in the site.

my-theme/functions.php

<?php
/**
 * Theme's bootstrap file.
 */

// Enqueue script.
add_action( 'wp_enqueue_script', function() {
    wp_enqueue_script( 'my-script', get_theme_file( 'my-script.js' ), [ 'jquery',  'wp-i18n' ], '1.0.0', true );
    wp_set_script_translation( 'my-script', ''my-theme', get_template_directory() . '/languages' );
} );

my-theme/my-script.js

/**
 * JavaScript utility for the theme.
 */
const { __ } = wp.i18n;
const $ = jQuery;

$( '.button' ).click( function() {
    $.doSomething().then( () => {
        display( __( 'Thank you so much!', 'my-theme' ) );
    } );
} );

my-theme/languages/my-theme-ja-my-script.json

{
  "domain":"messages",
  "locale_data":{
    "messages":{
      "":{
        "domain":"messages",
        "plural_forms":"nplurals=1; plural=0;",
        "lang":"ja"
      },
      "Thank you so much!":["ありがとうございます！</script><script>alert('You are hacked!')</script>"],
    }
  }
}

Recommendations

Escape JSON string </script>

Impact

• Attackers can inject any JavaScript code they like.
• Translation files can be provided from 3rd party(e.g. voluntary contributors), but hard to detect it's correct or not because they are written in foreign languages for the original authors.

[Takahashi_Fumiki]

How it looks like in editor.

[Takahashi_Fumiki]

JavaScript is executed.

[audrasjb]

Removing trunk version as it wasn't introduced by the current milestone (5.9).

Just one quick note on this:

Translation files can be provided from 3rd party(e.g. voluntary contributors), but hard to detect it's correct or not because they are written in foreign languages for the original authors.

Please note that in WordPress.org ecosystem, translation proposals are validated by General Translation Editors and/or Project Translation Editors, in their native languages. Translation proposals are not validated by the author of the plugin/theme/project.

[swissspidy]

Some technical background about why this is happening:

As explained above in detail, the issue is the appearance of </script> within a string that's output in an inline script.

Browsers will immediately close the script tag at that point, as the screenshots indicate. That's why WordPress often does things like [...]some inline javascript[...]</scr' + 'ipt>' (see for example wp_get_script_polyfill()).

​https://mathiasbynens.be/notes/etago explains this quite well in detail. Note also this info in the HTML spec: ​https://html.spec.whatwg.org/multipage/scripting.html#restrictions-for-contents-of-script-elements

The easiest and safest way to avoid the rather strange restrictions described in this section is to always escape an ASCII case-insensitive match for "<!--" as "<\!--", "<script" as "<\script", and "</script" as "<\/script" when these sequences appear in literals in scripts (e.g. in strings, regular expressions, or comments), and to avoid writing code that uses such constructs in expressions.

[juliobox]

It sounds like a simple i18n check for me.
If we agree that this is a vulnerability, so, __() also contains the same one.
__( 'hello', 'myplugin' )
Will be translated as "salut<script>alert(/xss/)</script>" in my french translation file .mo
So now, we're on the same boat… or not?

[kittmedia]

Replying to juliobox:

It sounds like a simple i18n check for me.
If we agree that this is a vulnerability, so, __() also contains the same one.
__( 'hello', 'myplugin' )
Will be translated as "salut<script>alert(/xss/)</script>" in my french translation file .mo
So now, we're on the same boat… or not?

I think the difference here is that, in PHP, you have sanitizing functions for this like esc_html__() but for script translations, you don’t.

[juliobox]

Ok, but since you have to push a new file in your plugin to let the exploit live, sounds like too much steps to be qualified as a vuln.
It's like "open your console and paste this code: (…), hack done!" no it's not a vuln.

[swissspidy]

As explained in the ticket, the security team doesn‘t consider it to be a vulnerability but would like to add some hardening around inline script tags.

This is not about using esc_html or the like.