forked from WordPress/wordpress-develop
-
Notifications
You must be signed in to change notification settings - Fork 0
Fix inline script tag escaping #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
sirreal
wants to merge
18
commits into
trunk
Choose a base branch
from
fix/62797-inline-script-tag-escape-alternate
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
353fa40
Add test for unclosed script tag
sirreal adc0ab3
Escape script modifiable text
sirreal 1639b2b
Stop escaping slashes, json_encode dangerously
sirreal fe6434d
Use the tag processor to correctly extract script tag contents
sirreal ced9c0f
Use the tag processor to correctly set script tag contents
sirreal 4b194ef
Improve closing script tag test
sirreal f8df461
Fix failing tests
sirreal b26f45d
Add dangerous script escaping tests
sirreal 4b9aa42
Apply script tag escaping in case of <script
sirreal 4f6d9d3
Improve script tag check
sirreal 7372021
Improve comment about script tag contents
sirreal bafb5fe
Improve script tag test and sanitization
sirreal e60d592
Add and improve tests
sirreal 092eecf
Fix boolean => bool return type
sirreal ed22488
Fix and improve tag processor comments
sirreal 529afad
Improve and add tests
sirreal 981d8e1
Add tests for \r and \r\n
sirreal a96d8ac
Handle \r as tag name terminator
sirreal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3726,17 +3726,54 @@ public function set_modifiable_text( string $plaintext_content ): bool { | |||||
switch ( $this->get_tag() ) { | ||||||
case 'SCRIPT': | ||||||
/* | ||||||
* This is over-protective, but ensures the update doesn't break | ||||||
* out of the SCRIPT element. A more thorough check would need to | ||||||
* ensure that the script closing tag doesn't exist, and isn't | ||||||
* also "hidden" inside the script double-escaped state. | ||||||
* SCRIPT tag contents can be dangerous. | ||||||
* | ||||||
* It may seem like replacing `</script` with `<\/script` would | ||||||
* properly escape these things, but this could mask regex patterns | ||||||
* that previously worked. Resolve this by not sending `</script` | ||||||
* The text `</script>` could close the SCRIPT element prematurely. | ||||||
* | ||||||
* The text `<script>` could enter the "script data double escaped state", preventing the | ||||||
* SCRIPT element from closing as expected, for example: | ||||||
* | ||||||
* <script> | ||||||
* // If this "<!--" then "<script>" the closing tag will not be recognized. | ||||||
* </script> | ||||||
* <h1>This appears inside the preceding SCRIPT element.</h1> | ||||||
* | ||||||
* The relevant state transitions happen on text like: | ||||||
* 1. < | ||||||
* 2. / (optional) | ||||||
* 3. script (case-insensitive) | ||||||
* 4. One of the following characters: | ||||||
* - \t | ||||||
* - \n | ||||||
* - \r (\r and \r\n newlines are normalized to \n in HTML pre-processing) | ||||||
* - \f | ||||||
* - " " (U+0020 SPACE) | ||||||
* - / | ||||||
* - > | ||||||
* | ||||||
* @see https://html.spec.whatwg.org/multipage/parsing.html#script-data-double-escaped-state | ||||||
*/ | ||||||
if ( false !== stripos( $plaintext_content, '</script' ) ) { | ||||||
return false; | ||||||
if ( preg_match( '~</?script[\t\r\n\f />]~i', $plaintext_content ) ) { | ||||||
/* | ||||||
* JavaScript can be safely escaped. | ||||||
* Non-JavaScript script tags have unknown semantics. | ||||||
* | ||||||
* @todo consider applying to JSON and importmap script tags as well. | ||||||
*/ | ||||||
if ( $this->is_javascript_script_tag() ) { | ||||||
$plaintext_content = preg_replace_callback( | ||||||
'~<(/?)(s)(cript)([\t\r\n\f />])~i', | ||||||
static function ( $matches ) { | ||||||
$escaped_s_char = 's' === $matches[2] | ||||||
? '\u0073' | ||||||
: '\u0053'; | ||||||
return "<{$matches[1]}{$escaped_s_char}{$matches[3]}{$matches[4]}"; | ||||||
}, | ||||||
$plaintext_content | ||||||
); | ||||||
} else { | ||||||
return false; | ||||||
} | ||||||
} | ||||||
|
||||||
$this->lexical_updates['modifiable text'] = new WP_HTML_Text_Replacement( | ||||||
|
@@ -3794,6 +3831,123 @@ static function ( $tag_match ) { | |||||
return false; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Indicates if the currently matched tag is a JavaScript script tag. | ||||||
* | ||||||
* @see https://html.spec.whatwg.org/multipage/scripting.html#prepare-the-script-element | ||||||
* | ||||||
* @since {WP_VERSION} | ||||||
sirreal marked this conversation as resolved.
Show resolved
Hide resolved
sirreal marked this conversation as resolved.
Show resolved
Hide resolved
sirreal marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The @SInCE tag contains a placeholder '{WP_VERSION}' instead of an actual version number. This should be replaced with the specific WordPress version when this feature is released.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
* | ||||||
* @return bool True if the script tag will be evaluated as JavaScript. | ||||||
*/ | ||||||
public function is_javascript_script_tag(): bool { | ||||||
if ( 'SCRIPT' !== $this->get_tag() || $this->get_namespace() !== 'html' ) { | ||||||
return false; | ||||||
} | ||||||
|
||||||
/* | ||||||
* > If any of the following are true: | ||||||
* > - el has a type attribute whose value is the empty string; | ||||||
* > - el has no type attribute but it has a language attribute and that attribute's | ||||||
* > value is the empty string; or | ||||||
* > - el has neither a type attribute nor a language attribute, | ||||||
* > then let the script block's type string for this script element be "text/javascript". | ||||||
*/ | ||||||
$type_attr = $this->get_attribute( 'type' ); | ||||||
$language_attr = $this->get_attribute( 'language' ); | ||||||
|
||||||
if ( true === $type_attr || '' === $type_attr ) { | ||||||
return true; | ||||||
} | ||||||
if ( | ||||||
null === $type_attr && ( | ||||||
true === $language_attr || | ||||||
'' === $language_attr || | ||||||
null === $language_attr | ||||||
) | ||||||
) { | ||||||
return true; | ||||||
} | ||||||
|
||||||
/* | ||||||
* > Otherwise, if el has a type attribute, then let the script block's type string be | ||||||
* > the value of that attribute with leading and trailing ASCII whitespace stripped. | ||||||
* > Otherwise, el has a non-empty language attribute; let the script block's type string | ||||||
* > be the concatenation of "text/" and the value of el's language attribute. | ||||||
*/ | ||||||
$type_string = $type_attr ? trim( $type_attr, " \t\f\r\n" ) : "text/{$language_attr}"; | ||||||
|
||||||
/* | ||||||
* > If the script block's type string is a JavaScript MIME type essence match, then | ||||||
* > set el's type to "classic". | ||||||
* | ||||||
* > A string is a JavaScript MIME type essence match if it is an ASCII case-insensitive | ||||||
* > match for one of the JavaScript MIME type essence strings. | ||||||
* | ||||||
* > A JavaScript MIME type is any MIME type whose essence is one of the following: | ||||||
* > | ||||||
* > - application/ecmascript | ||||||
* > - application/javascript | ||||||
* > - application/x-ecmascript | ||||||
* > - application/x-javascript | ||||||
* > - text/ecmascript | ||||||
* > - text/javascript | ||||||
* > - text/javascript1.0 | ||||||
* > - text/javascript1.1 | ||||||
* > - text/javascript1.2 | ||||||
* > - text/javascript1.3 | ||||||
* > - text/javascript1.4 | ||||||
* > - text/javascript1.5 | ||||||
* > - text/jscript | ||||||
* > - text/livescript | ||||||
* > - text/x-ecmascript | ||||||
* > - text/x-javascript | ||||||
* | ||||||
* @see https://mimesniff.spec.whatwg.org/#javascript-mime-type-essence-match | ||||||
* @see https://mimesniff.spec.whatwg.org/#javascript-mime-type | ||||||
*/ | ||||||
switch ( strtolower( $type_string ) ) { | ||||||
case 'application/ecmascript': | ||||||
case 'application/javascript': | ||||||
case 'application/x-ecmascript': | ||||||
case 'application/x-javascript': | ||||||
case 'text/ecmascript': | ||||||
case 'text/javascript': | ||||||
case 'text/javascript1.0': | ||||||
case 'text/javascript1.1': | ||||||
case 'text/javascript1.2': | ||||||
case 'text/javascript1.3': | ||||||
case 'text/javascript1.4': | ||||||
case 'text/javascript1.5': | ||||||
case 'text/jscript': | ||||||
case 'text/livescript': | ||||||
case 'text/x-ecmascript': | ||||||
case 'text/x-javascript': | ||||||
return true; | ||||||
|
||||||
/* | ||||||
* > Otherwise, if the script block's type string is an ASCII case-insensitive match for | ||||||
* > the string "module", then set el's type to "module". | ||||||
* | ||||||
* A module is evaluated as JavaScript. | ||||||
*/ | ||||||
case 'module': | ||||||
return true; | ||||||
} | ||||||
|
||||||
/* | ||||||
* > - Otherwise, if the script block's type string is an ASCII case-insensitive match for | ||||||
* > the string "importmap", then set el's type to "importmap". | ||||||
* | ||||||
* An importmap is JSON and not evaluated as JavaScript. This case is not handled here. | ||||||
*/ | ||||||
|
||||||
/* | ||||||
* > Otherwise, return. (No script is executed, and el's type is left as null.) | ||||||
*/ | ||||||
return false; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Updates or creates a new attribute on the currently matched tag with the passed value. | ||||||
* | ||||||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This TODO comment suggests unfinished work. Consider either implementing the suggested feature for JSON and importmap script tags or removing this comment if it's not planned for this release.
Copilot uses AI. Check for mistakes.