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

[machineitsvcs]

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

[machineitsvcs]

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

[machineitsvcs]

Any update on this?

[costdev]

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.

[SergeyBiryukov]

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.

[machineitsvcs]

I believe the main consideration to note here, whether or not the @ is used, is that the order of checking wp-config-sample.php and wp-config.php should be swapped, as most requests for a WordPress site would be for completed installations where wp-config.php exists, and therefore checking if wp-config-sample.php exists when wp-config.php already exists would produce the message that the wp-config-sample.php doesn't exists instead of the message that wp-config.php already exists.

Unless your concern is that the message stating wp-config.php exists instructs that the wp-config.php be deleted first, and that this would result in a broken installation if wp-config-sample.php also did not exist, requiring the wp-config-sample.php file be re-uploaded, which is already presented in a message if the file is missing anyway, so the messaging wouldn't even have to change.

Regardless, in my opinion, it just seems that logically it should consider if wp-config.php exists first, and only check if wp-config-sample.php exists if not. Although the improved performance from removing the two additional file_exists calls in the current version would be negligable, it should still be seen as best practice in most use-cases.

As for the security situation I mentioned, I was referring specifically just to restricting open_basedir to not include the directory above the web directory (which shouldn't need to be explained), and that such a restriction, in combination with the deletion of wp-config-sample.php file, would result in the PHP warning message in the log. That is all. Deleting the wp-config-sample.php file in and of itself is not necessarily a "security measure" - although deleting the file on a completed installation does reduce the unnecessary overhead from requests to this file, which subsequently loads the wp-settings.php file, which loads numerous other functions for no reason, to inevitably return a "Database Error - Error establishing a database connection" page, whereas on a completed installation the wp-config.php returns nothing (except a 200 response) if it is even publicly accessible (I personally block public access to wp-config.php using .htaccess). However, most do not take any similar steps to restrict public access to the wp-config-sample.php file if it still exists, as they assume like you did that it's a non-issue. Which isn't wrong exactly, but I believe it could definitely be improved upon, but that's for another discussion I guess. Hopefully this at least helps to explain why someone, like me, would delete the wp-config-sample.php file, and be plagued by the persistent error log messages/warnings due to bad actors trying to probe the setup-config.php file on my sites with the current file check order used.