Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#53830 closed defect (bug) (fixed)

Default filters try to create nonce during installation before options table exists

Reported by: schlessera's profile schlessera Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.8.1 Priority: normal
Severity: normal Version: 5.7
Component: Upgrade/Install Keywords: good-first-bug has-testing-info has-patch commit fixed-major
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Similar to #39047, there's a new instance where a default filter tries to use wp_create_nonce() during WP installation, which throws an error as the wp_options table does not yet exist.

The filter is adding the default scripts on 'init', and the offending code is found in wp-includes/script-loader.php:

<?php
$user_id = isset( $_GET['user_id'] ) ? (int) $_GET['user_id'] : 0;
did_action( 'init' ) && $scripts->localize(
        'user-profile',
        'userProfileL10n',
        array(
                'user_id' => $user_id,
                'nonce'   => wp_create_nonce( 'reset-password-for-' . $user_id ),
        )
);

It was introduced via [50129].

As suggested by @swissspidy, the wp_create_nonce() call should be wrapped like the following:

<?php
( wp_installing() && ! is_multisite() ) ? '' : wp_create_nonce(..)

Attachments (2)

53830.patch (521 bytes) - added by sanketchodavadiya 3 years ago.
I have made patch as @schlessera suggested and tested, its working as expected and remove warnining
53830-test.gif (8.4 MB) - added by hellofromTonya 3 years ago.
Test Report: (a) Reproduced bug. (b) Tested 53830.patch - resolves issue ✅

Change History (13)

#1 @swissspidy
3 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

#2 @SergeyBiryukov
3 years ago

  • Description modified (diff)
  • Keywords good-first-bug added
  • Milestone changed from Future Release to 5.9

#3 @SergeyBiryukov
3 years ago

  • Milestone changed from 5.9 to 5.8.1

Just noting that this appears to also happen when running unit tests, see this recent test run for example.

Moving to 5.8.1, as this was a recent change in 5.7 and the fix seems simple enough.

#4 @SergeyBiryukov
3 years ago

This can also be reproduced in regular installation:

  1. Create a wp-config.php without the key/salt constants filled in.
  2. Open /wp-admin/install.php.
  3. At the bottom of that page, there is a wp_print_scripts() call.
  4. wp_default_scripts() calls wp_create_nonce().
  5. wp_create_nonce() calls wp_hash().
  6. wp_hash() calls wp_salt().
  7. wp_salt() tries to save the key/salt values in the database and fails, as the table does not exist yet.

@sanketchodavadiya
3 years ago

I have made patch as @schlessera suggested and tested, its working as expected and remove warnining

@hellofromTonya
3 years ago

Test Report: (a) Reproduced bug. (b) Tested 53830.patch - resolves issue ✅

#5 @hellofromTonya
3 years ago

  • Keywords has-testing-info added

Test Report

Env

  • WordPress: 5.7.2 and 5.8
  • OS: macOS Big Sur 11.5
  • Localhost: Local (wp) with PHP 8.0, nginx, MySQL 8.0.16
  • Browser: Chrome

Test Steps

  • Step 1: Create a new site
  • Step 2: Open the wp-config.php file in your favorite editor
  • Step 3: Change the salt keys to empty string
    define( 'AUTH_KEY',         '' );
    define( 'SECURE_AUTH_KEY',  '' );
    define( 'LOGGED_IN_KEY',    '' );
    define( 'NONCE_KEY',        '' );
    define( 'AUTH_SALT',        '' );
    define( 'SECURE_AUTH_SALT', '' );
    define( 'LOGGED_IN_SALT',   '' );
    define( 'NONCE_SALT',       '' );
    
  • Step 4: Open the wp-admin/install.php file in your browser (e.g. http://newinstall.local/wp-admin/install.php where my test site's is http://newinstall.local
    • Step 4A: If message Already Installed appears, delete (drop) the tables in your local site's database. And then refresh the browser.

Notice the error:

WordPress database error: [Table 'local.wp_options' doesn't exist]

  • Step 5: Apply the patch.
  • Step 6: Refresh the browser.

Notice the error is gone and the installation can continue.

Results

See 53830-test.gif.

  • Reproduced the issue ✅
  • Applying 53830.patch resolves the issue ✅

#6 @hellofromTonya
3 years ago

  • Keywords has-patch commit added; needs-patch removed

@SergeyBiryukov thanks for the testing instructions!

@sanketchodavadiya thanks for the patch!

53830.patch resolves the issue (see testing report) ✅

Code review:

  • The patch implemented the suggested strategy ✅
  • The strategy is consistent with the other instances ✅
  • PHPCS/WPCS passes ✅
    composer lint ./src/wp-admin/install.php
    > @php ./vendor/squizlabs/php_codesniffer/bin/phpcs --report=summary,source './src/wp-admin/install.php'
    . 1 / 1 (100%)
    
  • Tests pass ✅
    npm run test:php -- --stop-on-failure
    
    Tests: 12462, Assertions: 57798, Skipped: 21.
    

Marking for commit.

#7 @SergeyBiryukov
3 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 51525:

Upgrade/Install: Avoid creating nonce during installation.

This avoids a "Table wp_options doesn't exist" database error when trying to create a nonce for password reset button.

When installing and using database-saved salts, wp_create_nonce() causes database errors as wp_salt() attempts to insert into the not-yet-created options table. Since authentication is not available during installation, we can safely skip creating a nonce.

Follow-up to [39684], [50129].

Props schlessera, swissspidy, sanketchodavadiya, hellofromTonya, SergeyBiryukov.
Fixes #53830.

#8 @SergeyBiryukov
3 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting to the 5.8 branch.

#9 @johnbillion
3 years ago

Follow-up: #53861

#10 @swissspidy
3 years ago

#53871 was marked as a duplicate.

#11 @peterwilsoncc
3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 51546:

Upgrade/Install: Avoid creating nonce during installation.

This avoids a "Table wp_options doesn't exist" database error when trying to create a nonce for password reset button.

When installing and using database-saved salts, wp_create_nonce() causes database errors as wp_salt() attempts to insert into the not-yet-created options table. Since authentication is not available during installation, we can safely skip creating a nonce.

Follow-up to [39684], [50129].

Props schlessera, swissspidy, sanketchodavadiya, hellofromTonya, SergeyBiryukov.
Merges [51525] to the 5.8 branch.
Fixes #53830.

Note: See TracTickets for help on using tickets.