Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#54893 new defect (bug)

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

Reported by: takahashi_fumiki's profile Takahashi_Fumiki Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: General Keywords: needs-patch
Focuses: javascript Cc:

Description

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.

Attachments (2)

__________2020-05-22_21_05_18.png (926.6 KB) - added by Takahashi_Fumiki 3 years ago.
How it looks like in editor.
__________2020-05-22_21.11.17.png (341.9 KB) - added by Takahashi_Fumiki 3 years ago.
JavaScript is executed.

Download all attachments as: .zip

Change History (8)

@Takahashi_Fumiki
3 years ago

How it looks like in editor.

@Takahashi_Fumiki
3 years ago

JavaScript is executed.

#1 @audrasjb
3 years ago

  • Version trunk deleted

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.

#2 @swissspidy
3 years ago

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.

#3 follow-up: @juliobox
3 years ago

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?

#4 in reply to: ↑ 3 @kittmedia
3 years ago

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.

#5 @juliobox
3 years ago

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.

#6 @swissspidy
3 years ago

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.

Note: See TracTickets for help on using tickets.