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: |
|
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:
- Create a small plugin or theme which uses
my-script.js
and using domainmy-theme
. - Create
my-theme.pot
and translate it inja.po
which includes string like</script><script>alert('Hacked!')<script>
. - Convert
ja.po
to JSONmy-theme-ja-my-script.json
via jed https://www.npmjs.com/package/jed - Execute
wp_set_script_translations( 'my-script', 'my-theme', get_template_directory() . '/languages' )
- 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)
Change History (8)
#1
@
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
@
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:
↓ 4
@
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
@
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.
How it looks like in editor.