Make WordPress Core

Opened 6 years ago

Last modified 3 weeks ago

#51947 new defect (bug)

When Customizer `setup_theme` action fails during wp-settings.php, WordPress crashes due to missing global $wp_locale

Reported by: Guss77 Owned by:
Priority: normal Milestone: Future Release
Component: Customize Version: 4.6
Severity: major Keywords: has-patch
Cc: Focuses:

Description

The wp-settings.php sets up the global $wp_locale (line 499 in WordPress 5.5.3) but before doing that it calls do_action( 'setup_theme' ); (line 478).

The problem is that WP_Customize_Manager::setup_theme() has several failure actions that call the class's wp_die() "wrapper" which - depending on the value of the messenger_channel might try to call wp_enqueue_scripts(), which will eventually call wp_localize_jquery_ui_datepicker() that expects $wp_locale to be already set. The result is a crash.

Here is one such stack trace:

PHP Fatal error:  Uncaught Error: Call to a member function is_rtl() on null in /var/www/html/wp-includes/script-loader.php:1684
Stack trace:
#0 /var/www/html/wp-includes/class-wp-hook.php(287): wp_localize_jquery_ui_datepicker('')
#1 /var/www/html/wp-includes/class-wp-hook.php(311): WP_Hook->apply_filters(NULL, Array)
#2 /var/www/html/wp-includes/plugin.php(478): WP_Hook->do_action(Array)
#3 /var/www/html/wp-includes/script-loader.php(2001): do_action('wp_enqueue_scri...')
#4 /var/www/html/wp-includes/class-wp-customize-manager.php(454): wp_enqueue_scripts()
#5 /var/www/html/wp-includes/class-wp-customize-manager.php(551): WP_Customize_Manager->wp_die(0, 'Non-existent ch...')
#6 /var/www/html/wp-includes/class-wp-hook.php(287): WP_Customize_Manager->setup_theme('')
#7 /var/www/html/wp-includes/class-wp-hook.php(311): WP_Hook->apply_filters(NULL, Array)
#8 /var/www/html/wp-includes/plugin.php(478): WP_Hook->do_action(Array)
#9 /var/www/html/wp-settings.php(478): do_action('setup_theme')
#10 /var/www/html/wp-config.php(97): req...
in /var/www/html/wp-includes/script-loader.php on line 1684, referer: https://somesite.com/

Moving the do_action( 'setup_theme' ); line down a few lines until after the local has been setup, and just before loading the active theme's function.php file (which is arguably where it was supposed to be in the first place) solves the problem.

Change History (9)

#1 @dlh
6 years ago

  • Keywords reporter-feedback added
  • Version 5.5.3

Thanks for the report, @Guss77!

Can you provide steps to replicate this behavior?

#2 @Guss77
6 years ago

For me it happens when the customizer is trying to output the "Non-existent changeset UUID" error message, and this happened on my multi-site with the MU Domain Mapping plugin when accessing the customizer in a site where the primary domain name is not a sub-domain of the multisite "DOMAIN_CURRENT_SITE".

#3 follow-up: @dlh
6 years ago

In my testing so far, I'm unable to replicate that fatal error. The script exits as expected.

wp_localize_jquery_ui_datepicker() won't attempt to use the $wp_locale global unless the jquery-ui-datepicker script is enqueued — see https://github.com/WordPress/wordpress-develop/blob/901909bf2cc79333d424628cef2064770b4b839b/src/wp-includes/script-loader.php#L1666-L1671.

Is a plugin enqueuing the datepicker? If so, when in the bootstrap cycle is it being enqueued, and how?

#4 in reply to: ↑ 3 @Guss77
6 years ago

Replying to dlh:

Is a plugin enqueuing the datepicker? If so, when in the bootstrap cycle is it being enqueued, and how?

very likely - I have several plugins installed that call wp_enqueue_script('jquery-ui-datepicker') or wp_register_script() with jquery-ui-datepicker as a dependency.

  • WordPress File Upload: calls wp_enqueue_script('jquery-ui-datepicker') from admin_print_scripts hook.
  • Contact Form 7: calls wp_enqueue_script('jquery-ui-datepicker') from the wp_enqueue_scripts hook.
  • Woo Order Export Lite: calls wp_enqueue_script('jquery-ui-datepicker') from admin_enqueue_scripts hook.

I think maybe the Contact Form 7 plugin is the issue, but I can list the plugins calling wp_register_script() if that is not it.

#5 @dlh
6 years ago

  • Keywords needs-patch added; reporter-feedback removed
  • Milestone Awaiting Review5.7
  • Version4.6

No need for the list of plugins, @Guss77. I can replicate simply by enqueuing the datepicker on wp_enqueue_scripts, since WP_Customize_Manager invokes it before exiting: https://github.com/WordPress/wordpress-develop/blob/668581d0f3be00c0babe6929f97ecce654507843/src/wp-includes/class-wp-customize-manager.php#L454

Moving the timing of setup_theme is going to be a non-starter for backwards-compatibility concerns, I suspect, but the I18N or theme component maintainers might have ideas for doing so. Adding safety checks to wp_localize_jquery_ui_datepicker() might be a less-risky approach. Let's see whether we can work something out in 5.7.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


6 years ago

#7 @johnbillion
6 years ago

  • Milestone 5.7Future Release

Bumping, pending ideas on how to fix this and a patch.

#8 @lambagency
3 weeks ago

Still reproducible on WordPress 7.0.4 / PHP 8.4.

The failing line has moved. is_rtl() was the first dereference on 5.5.3; on 7.0.4 it is
array_values( $wp_locale->month ) at script-loader.php:2020, so the error now reads:

Uncaught TypeError: array_values(): Argument #1 ($array) must be of type array, null given

Anyone searching that message will not find this ticket.

One aspect that may not be clear from the original report: this is reachable by unauthenticated front-end requests, not only from the admin Customizer. _wp_customize_include() instantiates WP_Customize_Manager whenever $_GETcustomize_changeset_uuid is set, with no capability check at that point, so any anonymous GET carrying a non-existent UUID plus customize_messenger_channel` reaches the same path.

It needs neither a valid changeset nor a login. On one site it fires most days against ordinary front-end URLs, from many different addresses, because an old preview URL is still being re-fetched by crawlers. Nobody there has opened the Customizer in months. The impact per request is small, one fatal and no data exposure, but nothing about the trigger is privileged.

The safety check @dlh suggested in comment:5 still looks like the right shape.

This ticket was mentioned in PR #13123 on WordPress/wordpress-develop by @wprashed.


3 weeks ago
#9

  • Keywords has-patch added; needs-patch removed

## Trac Ticket
https://core.trac.wordpress.org/ticket/51947

## Description

Prevents a fatal error when wp_localize_jquery_ui_datepicker() is called before the global $wp_locale has been initialized in wp-settings.php.

### The Problem

In wp-settings.php, the bootstrap order is:

  1. do_action('setup_theme') fires (line 705)
  2. Locale is loaded and $wp_locale = new WP_Locale() is created (line 728)

WP_Customize_Manager::setup_theme() hooks into the setup_theme action. When it encounters an error (e.g., non-existent changeset UUID, invalid theme), its wp_die() wrapper calls wp_enqueue_scripts() when a messenger_channel is present. This triggers wp_localize_jquery_ui_datepicker(), which attempts to access properties on the null $wp_locale:

array_values( $wp_locale->month )        // TypeError on PHP 8+
$wp_locale->is_rtl()                     // Fatal: Call to member function on null

### Security Surface

As noted by @lambagency in comment:8, this is reachable by unauthenticated front-end requests. _wp_customize_include() instantiates WP_Customize_Manager whenever $_GET['customize_changeset_uuid'] is set, with no capability check at that point. Any anonymous GET request carrying a non-existent UUID plus customize_messenger_channel reaches the fatal error path.

### The Fix

Adds an instanceof WP_Locale guard in wp_localize_jquery_ui_datepicker() to bail out gracefully when $wp_locale has not been initialized yet. This is the approach suggested by @dlh in comment:5.

function wp_localize_jquery_ui_datepicker() {
     global $wp_locale;

     if ( ! wp_script_is( 'jquery-ui-datepicker', 'enqueued' ) ) {
         return;
     }

+    if ( ! ( $wp_locale instanceof WP_Locale ) ) {
+        return;
+    }
+
     // Convert the PHP date format into jQuery UI's format.

### How to Reproduce

  1. Add to any plugin: add_action('wp_enqueue_scripts', function() { wp_enqueue_script('jquery-ui-datepicker'); });
  2. Visit any front-end URL with query parameters: ?customize_changeset_uuid=nonexistent&customize_messenger_channel=test
  3. Observe the fatal error (with WP_DEBUG enabled)

### Why instanceof instead of null check

Using instanceof WP_Locale rather than ! $wp_locale or null === $wp_locale ensures the guard is robust against any non-object value that might end up in the global, not just null.

Props dlh for the suggested approach.
Fixes #51947.

Note: See TracTickets for help on using tickets.