Add an early exit for files with _deprecated_file() calls

[SergeyBiryukov]

Background: #35835

[49619] added an early exit when calling the wp-includes/rss-functions.php file directly to avoid unnecessary errors in server logs.

There are ~30 other files in core with _deprecated_file() calls so it seems like a good idea to have a consistent policy for the other instances.

That said, it's not limited to _deprecated_file(), as noted in comment:3:ticket:35835 and comment:1:ticket:35835:

You'll get a similar error if you access wp-includes/default-filters.php or any number of others too potentially.

So perhaps other files that can clutter server logs with unnecessary errors would benefit from an early exit too.

[ayeshrajans]

This is probably not an ideal place to discuss this, but I'd like to raise a concern I had in my mind for a long time.

In WordPress, we have files that we can call directly from the browser (for example wp-admin/options-writing.php), and ones that we cannot (such as wp-blog-header.php). I totally agree that we need some sort of consistency, because at the moment, it is not clear which files are meant to be entry points, and which are to be included.

I wonder if there was any discussion to move the include files to where they belong, or rename them (for example to .inc in Drupal style), so they can be blocked from web server configuration and save ourselves of the early-exit calls, while also getting some potential opcache benefits?

[zodiac1978]

As a first step I checked the 30 search results which are using _deprecated_file():

These files are all resulting in a full path disclosure if display_errors is enabled:

wp-admin/admin-functions.php
wp-admin/custom-background.php
wp-admin/custom-header.php
wp-admin/includes/class-wp-upgrader-skins.php
wp-admin/upgrade-functions.php
wp-includes/class-feed.php
wp-includes/class-json.php
wp-includes/class-oembed.php
wp-includes/class-snoopy.php
wp-includes/class-wp-customize-control.php
wp-includes/class-wp-feed-cache.php
wp-includes/customize/class-wp-customize-new-menu-control.php
wp-includes/customize/class-wp-customize-new-menu-section.php
wp-includes/date.php
wp-includes/embed-template.php
wp-includes/locale.php
wp-includes/registration-functions.php
wp-includes/registration.php
wp-includes/rss-functions.php
wp-includes/rss.php
wp-includes/session.php
wp-includes/spl-autoload-compat.php
wp-includes/theme-compat/comments.php
wp-includes/theme-compat/footer.php
wp-includes/theme-compat/header.php
wp-includes/theme-compat/sidebar.php

PHPMailer is checking via function_exists first:

wp-includes/class-phpmailer.php

Nut sure why this is not reproducing the FPD:

wp-includes/class-smtp.php

This file has check and uses _deprecated_file() only if a my-hacks.php file exists:

wp-includes/load.php

And the 30th search result is just the function itself:

wp-includes/functions.php

And for completeness I would like to point again to the issue on the Health Check plugin to check for display_errors:
​https://github.com/WordPress/health-check/issues/370

because we recommend to disable this setting:
​https://make.wordpress.org/core/handbook/testing/reporting-security-vulnerabilities/#why-are-there-path-disclosures-when-directly-loading-certain-files

[SergeyBiryukov]

Thanks for the list of files! Just noting that this is not so much about full path disclosure specifically (which is covered by the ​handbook link above), but more about just preventing unnecessary errors in server logs.

[zodiac1978]

Just noting that this is not so much about full path disclosure specifically

I understand, but then we need to discuss our recommended path here.

The error log mentioned in the first ticket could be generated through testing tools, like wpcheck, which are just taking one of those files to check for full path disclosure:

For example: ​https://github.com/sergejmueller/wpcheck/blob/50fea1c1fe9b46d3fda8c2dae3b2214e9c0f5671/lib/rules/fpd-vulnerability.js#L32

If we fix it in one file, these tools will change to another file and generate unnecessary error logs again.

I think we have two (or three) possible solutions here:

• 1. Fix every single file to not show any errors on direct access.

• 2a. Force disabling display_errors per default (maybe not if WP_DEBUG is true)

• 2b. Add a check in Site Health to inform the user that display_errors is on.

If we have a decision on the path, then we could go forward.

Maybe @clorith can add an opinion on the Site Health idea?

[zodiac1978]

The idea from 2b about adding a check to Site Health is handled in #52652.

[SergeyBiryukov]

Related: #55944