diff --git a/src/wp-admin/includes/class-wp-automatic-updater.php b/src/wp-admin/includes/class-wp-automatic-updater.php index bc31e77427..eecc52a069 100644 --- a/src/wp-admin/includes/class-wp-automatic-updater.php +++ b/src/wp-admin/includes/class-wp-automatic-updater.php @@ -56,6 +56,54 @@ class WP_Automatic_Updater { return apply_filters( 'automatic_updater_disabled', $disabled ); } + /** + * Checks whether access to a given directory is allowed. + * + * This is used when detecting version control checkouts. Takes into account + * the PHP `open_basedir` restrictions, so that WordPress does not try to access + * directories it is not allowed to. + * + * @since 6.2.0 + * + * @param string $dir The directory to check. + * @return bool True if access to the directory is allowed, false otherwise. + */ + public function is_allowed_dir( $dir ) { + if ( is_string( $dir ) ) { + $dir = trim( $dir ); + } + + if ( ! is_string( $dir ) || '' === $dir ) { + _doing_it_wrong( + __METHOD__, + sprintf( + /* translators: %s: The "$dir" argument. */ + __( 'The "%s" argument must be a non-empty string.' ), + '$dir' + ), + '6.2.0' + ); + + return false; + } + + $open_basedir = ini_get( 'open_basedir' ); + + if ( empty( $open_basedir ) ) { + return true; + } + + $open_basedir_list = explode( PATH_SEPARATOR, $open_basedir ); + + foreach ( $open_basedir_list as $basedir ) { + if ( '' !== trim( $basedir ) && str_starts_with( $dir, $basedir ) ) { + return true; + } + } + + return false; + } + /** * Checks for version control checkouts. * @@ -102,7 +150,11 @@ class WP_Automatic_Updater { // Search all directories we've found for evidence of version control. foreach ( $vcs_dirs as $vcs_dir ) { foreach ( $check_dirs as $check_dir ) { - $checkout = @is_dir( rtrim( $check_dir, '\\/' ) . "/$vcs_dir" ); + if ( ! $this->is_allowed_dir( $check_dir ) ) { + continue; + } + + $checkout = is_dir( rtrim( $check_dir, '\\/' ) . "/$vcs_dir" ); if ( $checkout ) { break 2; } diff --git a/tests/phpunit/tests/admin/wpAutomaticUpdater.php b/tests/phpunit/tests/admin/wpAutomaticUpdater.php index 65bb39db2a..91ceaa1b38 100644 --- a/tests/phpunit/tests/admin/wpAutomaticUpdater.php +++ b/tests/phpunit/tests/admin/wpAutomaticUpdater.php @@ -572,4 +572,138 @@ class Tests_Admin_WpAutomaticUpdater extends WP_UnitTestCase { ), ); } + + /** + * Tests that `WP_Automatic_Updater::is_allowed_dir()` returns true + * when the `open_basedir` directive is not set. + * + * @ticket 42619 + * + * @covers WP_Automatic_Updater::is_allowed_dir + */ + public function test_is_allowed_dir_should_return_true_if_open_basedir_is_not_set() { + $this->assertTrue( self::$updater->is_allowed_dir( ABSPATH ) ); + } + + /** + * Tests that `WP_Automatic_Updater::is_allowed_dir()` returns true + * when the `open_basedir` directive is set and the path is allowed. + * + * Runs in a separate process to ensure that `open_basedir` changes + * don't impact other tests should an error occur. + * + * This test does not preserve global state to prevent the exception + * "Serialization of 'Closure' is not allowed" when running in + * a separate process. + * + * @ticket 42619 + * + * @covers WP_Automatic_Updater::is_allowed_dir + * + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_is_allowed_dir_should_return_true_if_open_basedir_is_set_and_path_is_allowed() { + // The repository for PHPUnit and test suite resources. + $abspath_parent = trailingslashit( dirname( ABSPATH ) ); + $abspath_grandparent = trailingslashit( dirname( $abspath_parent ) ); + + $open_basedir_backup = ini_get( 'open_basedir' ); + // Allow access to the directory one level above the repository. + ini_set( 'open_basedir', wp_normalize_path( $abspath_grandparent ) ); + + // Checking an allowed directory should succeed. + $actual = self::$updater->is_allowed_dir( wp_normalize_path( ABSPATH ) ); + + ini_set( 'open_basedir', $open_basedir_backup ); + + $this->assertTrue( $actual ); + } + + /** + * Tests that `WP_Automatic_Updater::is_allowed_dir()` returns false + * when the `open_basedir` directive is set and the path is not allowed. + * + * Runs in a separate process to ensure that `open_basedir` changes + * don't impact other tests should an error occur. + * + * This test does not preserve global state to prevent the exception + * "Serialization of 'Closure' is not allowed" when running in + * a separate process. + * + * @ticket 42619 + * + * @covers WP_Automatic_Updater::is_allowed_dir + * + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_is_allowed_dir_should_return_false_if_open_basedir_is_set_and_path_is_not_allowed() { + // The repository for PHPUnit and test suite resources. + $abspath_parent = trailingslashit( dirname( ABSPATH ) ); + $abspath_grandparent = trailingslashit( dirname( $abspath_parent ) ); + + $open_basedir_backup = ini_get( 'open_basedir' ); + // Allow access to the directory one level above the repository. + ini_set( 'open_basedir', wp_normalize_path( $abspath_grandparent ) ); + + // Checking a directory not within the allowed path should trigger an `open_basedir` warning. + $actual = self::$updater->is_allowed_dir( '/.git' ); + + ini_set( 'open_basedir', $open_basedir_backup ); + + $this->assertFalse( $actual ); + } + + /** + * Tests that `WP_Automatic_Updater::is_allowed_dir()` throws `_doing_it_wrong()` + * when an invalid `$dir` argument is provided. + * + * @ticket 42619 + * + * @covers WP_Automatic_Updater::is_allowed_dir + * + * @expectedIncorrectUsage WP_Automatic_Updater::is_allowed_dir + * + * @dataProvider data_is_allowed_dir_should_throw_doing_it_wrong_with_invalid_dir + * + * @param mixed $dir The directory to check. + */ + public function test_is_allowed_dir_should_throw_doing_it_wrong_with_invalid_dir( $dir ) { + $this->assertFalse( self::$updater->is_allowed_dir( $dir ) ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_is_allowed_dir_should_throw_doing_it_wrong_with_invalid_dir() { + return array( + // Type checks and boolean comparisons. + 'null' => array( 'dir' => null ), + '(bool) false' => array( 'dir' => false ), + '(bool) true' => array( 'dir' => true ), + '(int) 0' => array( 'dir' => 0 ), + '(int) -0' => array( 'dir' => -0 ), + '(int) 1' => array( 'dir' => 1 ), + '(int) -1' => array( 'dir' => -1 ), + '(float) 0.0' => array( 'dir' => 0.0 ), + '(float) -0.0' => array( 'dir' => -0.0 ), + '(float) 1.0' => array( 'dir' => 1.0 ), + 'empty string' => array( 'dir' => '' ), + 'empty array' => array( 'dir' => array() ), + 'populated array' => array( 'dir' => array( ABSPATH ) ), + 'empty object' => array( 'dir' => new stdClass() ), + 'populated object' => array( 'dir' => (object) array( ABSPATH ) ), + 'INF' => array( 'dir' => INF ), + 'NAN' => array( 'dir' => NAN ), + + // Ensures that `trim()` has been called. + 'string with only spaces' => array( 'dir' => ' ' ), + 'string with only tabs' => array( 'dir' => "\t\t" ), + 'string with only newlines' => array( 'dir' => "\n\n" ), + 'string with only carriage returns' => array( 'dir' => "\r\r" ), + ); + } }