Make WordPress Core

Changeset 55425


Ignore:
Timestamp:
02/26/2023 03:17:45 PM (3 months ago)
Author:
SergeyBiryukov
Message:

Upgrade/Install: Introduce WP_Automatic_Updater::is_allowed_dir() method.

As part of determining whether to perform automatic updates, WordPress checks if it is running within a version-controlled environment, recursively looking up the filesystem to the top of the drive, looking for a Subversion, Git, Mercurial, or Bazaar directory, erring on the side of detecting a VCS checkout somewhere.

This commit avoids a PHP warning if the open_basedir directive is in use and any of the directories checked in the process are not allowed:

is_dir(): open_basedir restriction in effect. File(/.git) is not within the allowed path(s)

Follow-up to [25421], [25700], [25764], [25835], [25859].

Props costdev, markjaquith, meyegui, dd32, arnolp, robin-labadie, hellofromTonya, afragen, pbiron, SergeyBiryukov.
Fixes #42619.

Location:
trunk
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/wp-admin/includes/class-wp-automatic-updater.php

    r55411 r55425  
    5858
    5959    /**
     60     * Checks whether access to a given directory is allowed.
     61     *
     62     * This is used when detecting version control checkouts. Takes into account
     63     * the PHP `open_basedir` restrictions, so that WordPress does not try to access
     64     * directories it is not allowed to.
     65     *
     66     * @since 6.2.0
     67     *
     68     * @param string $dir The directory to check.
     69     * @return bool True if access to the directory is allowed, false otherwise.
     70     */
     71    public function is_allowed_dir( $dir ) {
     72        if ( is_string( $dir ) ) {
     73            $dir = trim( $dir );
     74        }
     75
     76        if ( ! is_string( $dir ) || '' === $dir ) {
     77            _doing_it_wrong(
     78                __METHOD__,
     79                sprintf(
     80                    /* translators: %s: The "$dir" argument. */
     81                    __( 'The "%s" argument must be a non-empty string.' ),
     82                    '$dir'
     83                ),
     84                '6.2.0'
     85            );
     86
     87            return false;
     88        }
     89
     90        $open_basedir = ini_get( 'open_basedir' );
     91
     92        if ( empty( $open_basedir ) ) {
     93            return true;
     94        }
     95
     96        $open_basedir_list = explode( PATH_SEPARATOR, $open_basedir );
     97
     98        foreach ( $open_basedir_list as $basedir ) {
     99            if ( '' !== trim( $basedir ) && str_starts_with( $dir, $basedir ) ) {
     100                return true;
     101            }
     102        }
     103
     104        return false;
     105    }
     106
     107    /**
    60108     * Checks for version control checkouts.
    61109     *
     
    103151        foreach ( $vcs_dirs as $vcs_dir ) {
    104152            foreach ( $check_dirs as $check_dir ) {
    105                 $checkout = @is_dir( rtrim( $check_dir, '\\/' ) . "/$vcs_dir" );
     153                if ( ! $this->is_allowed_dir( $check_dir ) ) {
     154                    continue;
     155                }
     156
     157                $checkout = is_dir( rtrim( $check_dir, '\\/' ) . "/$vcs_dir" );
    106158                if ( $checkout ) {
    107159                    break 2;
  • trunk/tests/phpunit/tests/admin/wpAutomaticUpdater.php

    r54212 r55425  
    573573        );
    574574    }
     575
     576    /**
     577     * Tests that `WP_Automatic_Updater::is_allowed_dir()` returns true
     578     * when the `open_basedir` directive is not set.
     579     *
     580     * @ticket 42619
     581     *
     582     * @covers WP_Automatic_Updater::is_allowed_dir
     583     */
     584    public function test_is_allowed_dir_should_return_true_if_open_basedir_is_not_set() {
     585        $this->assertTrue( self::$updater->is_allowed_dir( ABSPATH ) );
     586    }
     587
     588    /**
     589     * Tests that `WP_Automatic_Updater::is_allowed_dir()` returns true
     590     * when the `open_basedir` directive is set and the path is allowed.
     591     *
     592     * Runs in a separate process to ensure that `open_basedir` changes
     593     * don't impact other tests should an error occur.
     594     *
     595     * This test does not preserve global state to prevent the exception
     596     * "Serialization of 'Closure' is not allowed" when running in
     597     * a separate process.
     598     *
     599     * @ticket 42619
     600     *
     601     * @covers WP_Automatic_Updater::is_allowed_dir
     602     *
     603     * @runInSeparateProcess
     604     * @preserveGlobalState disabled
     605     */
     606    public function test_is_allowed_dir_should_return_true_if_open_basedir_is_set_and_path_is_allowed() {
     607        // The repository for PHPUnit and test suite resources.
     608        $abspath_parent      = trailingslashit( dirname( ABSPATH ) );
     609        $abspath_grandparent = trailingslashit( dirname( $abspath_parent ) );
     610
     611        $open_basedir_backup = ini_get( 'open_basedir' );
     612        // Allow access to the directory one level above the repository.
     613        ini_set( 'open_basedir', wp_normalize_path( $abspath_grandparent ) );
     614
     615        // Checking an allowed directory should succeed.
     616        $actual = self::$updater->is_allowed_dir( wp_normalize_path( ABSPATH ) );
     617
     618        ini_set( 'open_basedir', $open_basedir_backup );
     619
     620        $this->assertTrue( $actual );
     621    }
     622
     623    /**
     624     * Tests that `WP_Automatic_Updater::is_allowed_dir()` returns false
     625     * when the `open_basedir` directive is set and the path is not allowed.
     626     *
     627     * Runs in a separate process to ensure that `open_basedir` changes
     628     * don't impact other tests should an error occur.
     629     *
     630     * This test does not preserve global state to prevent the exception
     631     * "Serialization of 'Closure' is not allowed" when running in
     632     * a separate process.
     633     *
     634     * @ticket 42619
     635     *
     636     * @covers WP_Automatic_Updater::is_allowed_dir
     637     *
     638     * @runInSeparateProcess
     639     * @preserveGlobalState disabled
     640     */
     641    public function test_is_allowed_dir_should_return_false_if_open_basedir_is_set_and_path_is_not_allowed() {
     642        // The repository for PHPUnit and test suite resources.
     643        $abspath_parent      = trailingslashit( dirname( ABSPATH ) );
     644        $abspath_grandparent = trailingslashit( dirname( $abspath_parent ) );
     645
     646        $open_basedir_backup = ini_get( 'open_basedir' );
     647        // Allow access to the directory one level above the repository.
     648        ini_set( 'open_basedir', wp_normalize_path( $abspath_grandparent ) );
     649
     650        // Checking a directory not within the allowed path should trigger an `open_basedir` warning.
     651        $actual = self::$updater->is_allowed_dir( '/.git' );
     652
     653        ini_set( 'open_basedir', $open_basedir_backup );
     654
     655        $this->assertFalse( $actual );
     656    }
     657
     658    /**
     659     * Tests that `WP_Automatic_Updater::is_allowed_dir()` throws `_doing_it_wrong()`
     660     * when an invalid `$dir` argument is provided.
     661     *
     662     * @ticket 42619
     663     *
     664     * @covers WP_Automatic_Updater::is_allowed_dir
     665     *
     666     * @expectedIncorrectUsage WP_Automatic_Updater::is_allowed_dir
     667     *
     668     * @dataProvider data_is_allowed_dir_should_throw_doing_it_wrong_with_invalid_dir
     669     *
     670     * @param mixed $dir The directory to check.
     671     */
     672    public function test_is_allowed_dir_should_throw_doing_it_wrong_with_invalid_dir( $dir ) {
     673        $this->assertFalse( self::$updater->is_allowed_dir( $dir ) );
     674    }
     675
     676    /**
     677     * Data provider.
     678     *
     679     * @return array[]
     680     */
     681    public function data_is_allowed_dir_should_throw_doing_it_wrong_with_invalid_dir() {
     682        return array(
     683            // Type checks and boolean comparisons.
     684            'null'                              => array( 'dir' => null ),
     685            '(bool) false'                      => array( 'dir' => false ),
     686            '(bool) true'                       => array( 'dir' => true ),
     687            '(int) 0'                           => array( 'dir' => 0 ),
     688            '(int) -0'                          => array( 'dir' => -0 ),
     689            '(int) 1'                           => array( 'dir' => 1 ),
     690            '(int) -1'                          => array( 'dir' => -1 ),
     691            '(float) 0.0'                       => array( 'dir' => 0.0 ),
     692            '(float) -0.0'                      => array( 'dir' => -0.0 ),
     693            '(float) 1.0'                       => array( 'dir' => 1.0 ),
     694            'empty string'                      => array( 'dir' => '' ),
     695            'empty array'                       => array( 'dir' => array() ),
     696            'populated array'                   => array( 'dir' => array( ABSPATH ) ),
     697            'empty object'                      => array( 'dir' => new stdClass() ),
     698            'populated object'                  => array( 'dir' => (object) array( ABSPATH ) ),
     699            'INF'                               => array( 'dir' => INF ),
     700            'NAN'                               => array( 'dir' => NAN ),
     701
     702            // Ensures that `trim()` has been called.
     703            'string with only spaces'           => array( 'dir' => '   ' ),
     704            'string with only tabs'             => array( 'dir' => "\t\t" ),
     705            'string with only newlines'         => array( 'dir' => "\n\n" ),
     706            'string with only carriage returns' => array( 'dir' => "\r\r" ),
     707        );
     708    }
    575709}
Note: See TracChangeset for help on using the changeset viewer.