Make WordPress Core

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: artpi's profile artpi 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

  1. Start writing a post in Gutenberg
  2. Enter code editor
  3. Paste following code

<p><!-- <script> </p>

   </p>

  1. Save
  2. Exit code editor into block editor
  3. Reload the page
  4. 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)

testing-without-patch.mp4 (1.2 MB) - added by shanemuir 3 weeks ago.
Testing without patch applied.
testing-with-patch.mp4 (3.6 MB) - added by shanemuir 3 weeks ago.
Testing with patch applied.

Change History (11)

#1 @ankitkumarshah
5 weeks ago

Reproduction Report

Description

This report validates whether the issue can be reproduced.

Environment

  • WordPress: 6.8-alpha-59593
  • PHP: 8.1.29
  • Server: nginx/1.16.0
  • Database: mysqli (Server: 8.0.16 / Client: mysqlnd 8.1.29)
  • Browser: Chrome 131.0.0.0
  • OS: macOS
  • Theme: Twenty Seventeen 3.8
  • MU Plugins: None activated
  • Plugins:
    • Gutenslider — The last WordPress slider you will ever need. 6.1.0
    • Test Plugin
    • Test Reports 1.2.0
    • WordPress Beta Tester 3.6.2

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

#2 @abcd95
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 @dilipbheda
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 @sainathpoojary
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

https://rioudcpuyg.ufs.sh/f/PL8E4NiPUWyO9PFMNgfEFACY0dej4LmhwrRp2qTtuWnBUoas

Last edited 5 weeks ago by sainathpoojary (previous) (diff)

#7 @jonsurrell
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

@shanemuir
3 weeks ago

Testing without patch applied.

@shanemuir
3 weeks ago

Testing with patch applied.

#9 @shanemuir
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

  1. ✅ 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.

Note: See TracTickets for help on using tickets.