Make WordPress Core

Opened 2 years ago

Last modified 8 days ago

#53298 new defect (bug)

Checking if wp-config-sample.php file exists before checking if wp-config.php exists

Reported by: machineitsvcs's profile machineitsvcs Owned by:
Milestone: Awaiting Review Priority: normal
Severity: trivial Version: 5.7.2
Component: Upgrade/Install Keywords: needs-patch dev-feedback
Focuses: administration, privacy, coding-standards Cc:

Description

Currently in WordPress core, wp-admin/setup-config.php checks if wp-config-sample.php file exists before checking if wp-config.php exists. If the sample file exists, it then checks if the wp-config.php file exists, and if so, suggests deletion if necessary. For security, some WordPress users may delete the sample file, and restrict open_basedir for directory above that of the web root directory. Because of these two cases, the current order produces the follow error:

PHP message: PHP Warning: file_exists(): open_basedir restriction in effect. File(/var/www/example/wp-config-sample.php) is not within the allowed path(s): (/var/www/example/web:/var/www/example/private:/var/www/example/tmp:/tmp:...) in /var/www/example/web/wp-admin/setup-config.php on line 46

If the check for existence of sample file could be moved after checking if wp-config.php exists, we could avoid this error and avoid checking if sample file exists if wp-config.php does and not checking both if they both do.

i.e. Moving the section commented Support wp-config-sample.php one level up, for the develop repo. to after the section commented Check if wp-config.php exists above the root directory but is not part of another installation. in wp-admin/setup-config.php

Change History (5)

#1 @SergeyBiryukov
2 years ago

  • Component changed from General to Upgrade/Install

#2 @machineitsvcs
2 years ago

It may also be worth prefacing the elseif condition file_exists( dirname( ABSPATH ) . '/wp-config-sample.php' ) with a @ to avoid any error should both the wp-config.php and wp-config-sample.php not exist and the parent directory be inaccessible, similar to what is already done for the wp-config.php when looking in parent directory.

i.e. using @file_exists( dirname( ABSPATH ) . '/wp-config-sample.php' ) instead

Last edited 2 years ago by machineitsvcs (previous) (diff)

#3 @machineitsvcs
19 months ago

Any update on this?

#4 follow-up: @costdev
8 months ago

  • Keywords dev-feedback added

Hi @machineitsvcs, welcome to Trac and thanks for opening this ticket, and providing some potential solutions!

As you said, if we reverse the conditions, then if wp-config.php and wp-config-sample.php don't exist in ABSPATH, the second wp-config-sample.php check will try to go up one level, and the warning is still shown. For that reason, I don't think swapping the order is the solution here.

@SergeyBiryukov The @ operator seems the most appropriate solution here as it's already there for the wp-config.php and wp-settings.php checks.

However, I know that we're trying to reduce the number of @ operators in Core, and adding a new one seems counter-productive to that effort. That said, file_exists() emits an E_WARNING, not a fatal error, and when considering consistency with other checks in this file, it may be acceptable to add a new @ operator here.

I'd appreciate your thoughts on how this might proceed.

#5 in reply to: ↑ 4 @SergeyBiryukov
8 days ago

Replying to costdev:

I'd appreciate your thoughts on how this might proceed.

WP_Automatic_Updater::is_allowed_dir() was recently introduced in [55425] / #42619 to check for an open_basedir restriction in the context of automatic updates.

It looks like this may be useful in other contexts too, so perhaps we could move that check into a separate function and make that method a wrapper?

That said, it might not be worth it if this is the only other place where that check is needed, so using @ seems fine.

Replying to machineitsvcs:

For security, some WordPress users may delete the sample file, and restrict open_basedir for directory above that of the web root directory.

I'm curious though, what kind of security enhancement does removing the wp-config-sample.php file provide? Unless I'm missing something, that file cannot be used in any way if wp-config.php exists already.

Note: See TracTickets for help on using tickets.