Make WordPress Core

Opened 11 months ago

Last modified 10 months ago

#38946 new enhancement

WP_Upgrader: Protection against deleting files in destination directory

Reported by: shivapoudel Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch dev-feedback
Focuses: Cc:


While creating a plugin I have to upload zip file and extract it in the custom destination directory with its inside main folder name just like how Theme_Upgrader and Plugin_Upgrader does.

While extending the class WP_Upgrader and using method install() like this:

 * Install a demo package.
 * @since 2.8.0
 * @since 3.7.0 The `$args` parameter was added, making clearing the update cache optional.
 * @access public
 * @param string $package The full local path or URI of the package.
 * @param array  $args {
 *     Optional. Other arguments for installing a demo package. Default empty array.
 *     @type bool $clear_update_cache Whether to clear the updates cache if successful.
 *                                    Default true.
 * }
 * @return bool|WP_Error True if the install was successful, false or a WP_Error object otherwise.
public function install( $package, $args = array() ) {

        $defaults = array(
                'clear_update_cache' => true,
        $parsed_args = wp_parse_args( $args, $defaults );


        add_filter( 'upgrader_source_selection', array( $this, 'check_package' ) );

        $this->run( array(
                'package' => $package,
                'destination' => WP_CONTENT_DIR . '/uploads/demo-packs',
                'clear_destination' => false, // Do not overwrite files.
                'protect_destination' => true, // Let me utilize this args in `install_package()` to fix this error, happy :)
                'clear_working' => true,
                'hook_extra' => array(
                        'type' => 'demo',
                        'action' => 'install',
        ) );

        remove_filter( 'upgrader_source_selection', array( $this, 'check_package' ) );

        if ( ! $this->result || is_wp_error( $this->result ) ) {
                return $this->result;

        return true;

Now always the zip file content are extracted into that destination directory but I want to index just like this where {uploaded-zip} is the zip file inside folder name:

WP_CONTENT_DIR . '/uploads/demo-packs/{uploaded-zip}'

And finally the detected issue is that the destination directory are no more protected just like theme and plugins directories does :)

 * Protection against deleting files in any important base directories.
 * Theme_Upgrader & Plugin_Upgrader also trigger this, as they pass the
 * destination directory (WP_PLUGIN_DIR / wp-content/themes) intending
 * to copy the directory into the directory, whilst they pass the source
 * as the actual files to copy.
$protected_directories = array( ABSPATH, WP_CONTENT_DIR, WP_PLUGIN_DIR, WP_CONTENT_DIR . '/themes' );

if ( is_array( $wp_theme_directories ) ) {
        $protected_directories = array_merge( $protected_directories, $wp_theme_directories );

if ( in_array( $destination, $protected_directories ) ) {
        $remote_destination = trailingslashit( $remote_destination ) . trailingslashit( basename( $source ) );
        $destination = trailingslashit( $destination ) . trailingslashit( basename( $source ) );

And this is where the magic begins by adding protect_destination args and fixing it:

if ( in_array( $destination, $protected_directories ) || $args['protect_destination'] ) {
        $remote_destination = trailingslashit( $remote_destination ) . trailingslashit( basename( $source ) );
        $destination = trailingslashit( $destination ) . trailingslashit( basename( $source ) );

Attachments (3)

class-wp-upgrader.php (33.1 KB) - added by shivapoudel 11 months ago.
Changeset in but fix
38946.patch (4.9 KB) - added by shivapoudel 11 months ago.
Fix for 38946
38946.diff (4.9 KB) - added by shivapoudel 11 months ago.
Fix for 38946 - Diff

Download all attachments as: .zip

Change History (7)

11 months ago

Changeset in but fix

#1 follow-up: @dd32
11 months ago

@shivapoudel Wouldn't it be possible to simply include the subdirectory in the destination arg?

That being said, having some way of triggering this specific code check for others, either through a filter or a parameter sounds like a good idea.

Would you be able to attach your changes as a `diff` file rather than the entire file? Please also make sure that any args added have defaults set correctly too.

11 months ago

Fix for 38946

11 months ago

Fix for 38946 - Diff

#2 @shivapoudel
11 months ago

  • Keywords has-patch added

#3 in reply to: ↑ 1 @shivapoudel
11 months ago

Replying to dd32:

It wouldn't be possible to simply guess the basename of package, strip .zip extension and set that match as subdirectory for destination arg. This is because it may result subdirectory like uploaded-demo-pack-{1|2|3|..} and inside zip folder the directory name is predefined and look like uploaded-demo-pack.

This above patch tries to implement protect_destination arg and have defaults set to false :)

Last edited 11 months ago by shivapoudel (previous) (diff)

#4 @shivapoudel
10 months ago

  • Keywords dev-feedback added
Note: See TracTickets for help on using tickets.