Skip to content

Commit 140b242

Browse files
sirrealspacedmonkey
authored andcommitted
HTML API: Prevent adding dangerous double-escape SCRIPT contents.
Prevent WP_Tag_Processor::set_modifiable_text() from allowing SCRIPT contents with "<script" like it does with "</script". Either of these sequences may affect the script element's close. Developed in WordPress#9560. Props jonsurrell, westonruter, dmsnell. See #63738. git-svn-id: https://develop.svn.wordpress.org/trunk@60706 602fd350-edb4-49c9-b593-d223f7449a82
1 parent 56835fa commit 140b242

File tree

2 files changed

+20
-8
lines changed

2 files changed

+20
-8
lines changed

src/wp-includes/html-api/class-wp-html-tag-processor.php

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3780,17 +3780,28 @@ public function set_modifiable_text( string $plaintext_content ): bool {
37803780

37813781
switch ( $this->get_tag() ) {
37823782
case 'SCRIPT':
3783-
/*
3783+
/**
37843784
* This is over-protective, but ensures the update doesn't break
3785-
* out of the SCRIPT element. A more thorough check would need to
3786-
* ensure that the script closing tag doesn't exist, and isn't
3787-
* also "hidden" inside the script double-escaped state.
3785+
* the HTML structure of the SCRIPT element.
3786+
*
3787+
* More thorough analysis could track the HTML tokenizer states
3788+
* and to ensure that the SCRIPT element closes at the expected
3789+
* SCRIPT close tag as is done in {@see ::skip_script_data()}.
37883790
*
3789-
* It may seem like replacing `</script` with `<\/script` would
3790-
* properly escape these things, but this could mask regex patterns
3791-
* that previously worked. Resolve this by not sending `</script`
3791+
* A SCRIPT element could be closed prematurely by contents
3792+
* like `</script>`. A SCRIPT element could be prevented from
3793+
* closing by contents like `<!--<script>`.
3794+
*
3795+
* The following strings are essential for dangerous content,
3796+
* although they are insufficient on their own. This trade-off
3797+
* prevents dangerous scripts from being sent to the browser.
3798+
* It is also unlikely to produce HTML that may confuse more
3799+
* basic HTML tooling.
37923800
*/
3793-
if ( false !== stripos( $plaintext_content, '</script' ) ) {
3801+
if (
3802+
false !== stripos( $plaintext_content, '</script' ) ||
3803+
false !== stripos( $plaintext_content, '<script' )
3804+
) {
37943805
return false;
37953806
}
37963807

tests/phpunit/tests/html-api/wpHtmlTagProcessorModifiableText.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,7 @@ public static function data_unallowed_modifiable_text_updates() {
490490
'Comment with --!>' => array( '<!-- this is a comment -->', 'Invalid but legitimate comments end in --!>' ),
491491
'SCRIPT with </script>' => array( '<script>Replace me</script>', 'Just a </script>' ),
492492
'SCRIPT with </script attributes>' => array( '<script>Replace me</script>', 'before</script id=sneak>after' ),
493+
'SCRIPT with "<script " opener' => array( '<script>Replace me</script>', '<!--<script ' ),
493494
);
494495
}
495496
}

0 commit comments

Comments
 (0)