Opened 5 weeks ago
Last modified 3 weeks ago
#62797 new defect (bug)
wp_add_inline_script does not properly escape '<!-- <script>' in contents
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 5.0 |
Component: | Editor | Keywords: | has-patch |
Focuses: | administration | Cc: |
Description
I am adding this issue in the "Editor" because this is how it's easiest to reproduce and how I stumbled onto it, but it is possible to replace the issue outside of editor context.
Reproduction path
- Start writing a post in Gutenberg
- Enter code editor
- Paste following code
<p><!-- <script> </p> </p>
- Save
- Exit code editor into block editor
- Reload the page
- Observe the editor crash and burn
Here is video of a reproduction on a fresh site:
https://screen.studio/share/ELc9d6zJ
What is going on?
block_editor_rest_api_preload
is using wp_add_inline_script to preload the currently edited block content into the page
https://github.com/WordPress/WordPress/blob/0086f4ba4080285c90c08854a5d085c76b6d5109/wp-includes/block-editor.php#L767
Because the <script> tag is hidden in an comment that has not been properly terminated, it is impossible to get out of the script tag now, and nothing gets rendered afterwards.
Tested up to WordPress version 6.7.1
Attachments (2)
Change History (11)
#2
@
5 weeks ago
Hey @artpi, Thanks for bringing this up.
I was able to reproduce the issue - Screencast
The problem occurs because wp_add_inline_script()
injects the block content inside a <script> tag for preloading. When there's an unclosed HTML comment containing <script> in the content, it breaks out of the string context since browsers parse this as the start of a new script tag, causing the editor to crash.
I will work on this to find a fix.
Additional Note: This happens with all variants of the script like <!-- <SCRIPT>
This ticket was mentioned in PR #8106 on WordPress/wordpress-develop by @dilipbheda.
5 weeks ago
#3
- Keywords has-patch added
#4
@
5 weeks ago
- Focuses administration added
- Keywords needs-testing added
@artpi, thank you for the report. I've addressed it with the attached PR.
@abcd95 @ankitkumarshah, could you please review the attached PR and let me know if you find any other issues?
Thanks!
#5
@
5 weeks ago
Hey @dilipbheda,
I have tested the PR. While trying to reproduce the issue, I encountered this warning. It might be related to a regex issue in this line
#7
@
5 weeks ago
A fix for all cases from the wp_add_inline_script
side will be difficult. It may be something the HTML API could handle.
The problem in this case is with the JSON encoding. Use of the correct flags should fix the problem, namely JSON_HEX_TAG
.
A good example to follow is this code used by script modules.
This ticket was mentioned in PR #8145 on WordPress/wordpress-develop by @jonsurrell.
3 weeks ago
#8
Fixes #62797 by correctly escaping JSON included inside JavaScript in a script
tag.
wp_add_inline_script
will correctly escape closing script tags </script>
which prevents script contents from breaking out of the script, it does not handle cases where some content may enter the HTML script data double escaped state and prevent the real script closing tag from closing the script tag as expected, leading to a broken document where the script tag remains open.
This can be tested by applying the patch and following the reproduction steps in #62797.
Trac ticket: https://core.trac.wordpress.org/ticket/62797
#9
@
3 weeks ago
- Keywords needs-testing removed
Test Report
Description
This report validates whether the indicated patch works as expected.
Patch tested: https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/8145.diff
Environment
- WordPress: 6.8-alpha-59274-src
- PHP: 8.3.15
- Server: Apache/2.4.62 (Unix) OpenSSL/3.4.0 PHP/8.3.15
- Database: mysqli (Server: 8.0.39 / Client: mysqlnd 8.3.15)
- Browser: Chrome 132.0.0.0
- OS: macOS
- Theme: Twenty Twenty-Five 1.0
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.0
Actual Results
- ✅ Issue resolved with patch.
Additional Notes
Only testing using @jonsurrell patch, let me know if we need to test the other patch too?
Supplemental Artifacts
Added videos of testing with and without patch applied.
Reproduction Report
Description
This report validates whether the issue can be reproduced.
Environment
Actual Results
✅ Error condition occurs (reproduced).
Hi @artpi,
Thank you for bringing this up. By following the provided steps, I reproduced the issue successfully.
Screencast
https://ppgtp1rtd3.ufs.sh/f/TnWMEUzoUd85Fb83cfTuejSJhDdioPMH6NAwRtYmfvQBT3W9