Make WordPress Core

Opened 4 years ago

Last modified 4 years ago

#50850 new defect (bug)

When the deactivate_plugins() function is called in the ABSPATH/wp-admin/includes/plugin.php file, the is_plugin_active_for_network() conditional tag always returns true when the active_sitewide_plugins sitemeta option is set to 1

Reported by: zenithcity's profile zenithcity Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.4.2
Component: Plugins Keywords: needs-patch
Focuses: multisite Cc:

Description

On multisite during the process of activating a plugin across the network (sitewide plugin activation), I noticed that if the activation process is not successful or if errors are triggered, the `active_sitewide_plugins' sitemeta option will be set to 1 and this causes the is_plugin_active_for_network() conditional tag to return true when called within the deactivate_plugins function, which is so untrue.

Also, when on the admin or network plugins.php screen, the deactivate_plugins() function triggers the following error becuase the $plugins = get_site_option( 'active_sitewide_plugins' ) code doesn't return the correct type (array):

 Fatal error: Uncaught Error: Cannot unset string offsets in ABSPATH\wp-admin\includes\plugin.php on line 779

Futhermore, after exploring the error, I found that since the active_sitewide_plugins sitemeta option is set to 1 and the array_keys() function is used to extract the plugin basenames into a new array and merged with non-sitewide activated plugins, then when used in the foreach() loop, this will always return true.

<?php
/**
 * @see ABSPATH/wp-admin/includes/plugin.php#L1047
 */
function validate_active_plugins() {
    $plugins = get_option( 'active_plugins', array() );
    // Validate vartype: array.
    if ( ! is_array( $plugins ) ) {
        update_option( 'active_plugins', array() );
        $plugins = array();
    }

    if ( is_multisite() && current_user_can( 'manage_network_plugins' ) ) {
        $network_plugins = (array) get_site_option( 'active_sitewide_plugins', array() );

        // If the 'active_sitewide_plugins' option is set to 1, 
        // then the $network_plugins var will contain 0 as the only elements 
        $plugins         = array_merge( $plugins, array_keys( $network_plugins ) );
    }

    if ( empty( $plugins ) ) {
        return array();
    }

    $invalid = array();

    // Invalid plugins get deactivated.
    foreach ( $plugins as $plugin ) {
        $result = validate_plugin( $plugin );
        if ( is_wp_error( $result ) ) {
            $invalid[ $plugin ] = $result;
            deactivate_plugins( $plugin, true );
        }
    }
    return $invalid;
}

I also ran a test with the is_plugin_active_for_network() conditional tag using the in_array() function to replace the isset() function in other to see if things would work as normal, and the test was passed:

<?php
function is_plugin_active_for_network( $plugin ) {
    if ( ! is_multisite() ) {
        return false;
    }

    // Need fix: the $plugins var need to be type-cast into an array
    $plugins = get_site_option( 'active_sitewide_plugins' );

    //if ( isset( $plugins[ $plugin ] ) ) {

    // Note: If the $plugins var is not an array then we have to 
    // type-cast it into an array
    if ( in_array( $plugin, (array) $plugins, true ) ) {
        return true;
    }

    return false;
}

File:

ABSPATH/wp-admin/includes/plugin.php

Attachments (1)

50850.diff (1.1 KB) - added by zenithcity 4 years ago.

Download all attachments as: .zip

Change History (5)

@zenithcity
4 years ago

#1 @SergeyBiryukov
4 years ago

  • Component changed from Options, Meta APIs to Plugins
  • Focuses multisite added

#2 in reply to: ↑ description ; follow-up: @SergeyBiryukov
4 years ago

Hi there, welcome to WordPress Trac! Thanks for the report and the patch.

On multisite during the process of activating a plugin across the network (sitewide plugin activation), I noticed that if the activation process is not successful or if errors are triggered, the `active_sitewide_plugins' sitemeta option will be set to 1

I could not reproduce a path in core where active_sitewide_plugins would be set to 1. It's only updated in activate_plugin() and deactivate_plugins(), with both instances receiving an array.

Adding some protection against an invalid value sounds good in general, but this doesn't seem any different from accidentally setting an invalid value for any other option that should always be an array: active_plugins, recently_activated, recently_edited, uninstall_plugins, *_paused_extensions, recovery_keys, theme_mods_*, nav_menu_options, sticky_posts, to name a few.

So far this seems like a plugin error. If there is a path in core where setting active_sitewide_plugins to 1 can be easily reproduced with a specific example, demonstrating that would probably be a good idea for the ticket to move forward.

#3 in reply to: ↑ 2 @zenithcity
4 years ago

Hi Sergey

I could not reproduce a path in core where active_sitewide_plugins would be set to 1. It's only updated > in activate_plugin()​ and deactivate_plugins()​, with both instances receiving an array.

Yes, the instance where 1 is being displayed as the value to the active_sitewide_plugins column was a clear cut surprise to me. Even if errors should occur that should not be happening. The source plugin I was testing with, I deliberately created and triggered the error during the plugin activation process to see what happens thereafter.

I created the ticket because the test plugin just triggers errors, no instance of updating/adding any value anywhere, and the other plugins I have installed are Hello and Akismet, both not activated.

Currently, after modifying the plugin and updating the 'active_sitewide_plugins' option to an empty serialized array string, even the errors, I cannot reproduce the instance where 1 is set to as the value.

This is strange and unusual. Interesting!

#4 @SergeyBiryukov
4 years ago

  • Severity changed from critical to normal
Note: See TracTickets for help on using tickets.