Make WordPress Core

Opened 7 years ago

Last modified 8 months ago

#42527 new defect (bug)

Admin menus: PHP warning "open_basedir restriction..." when `_wp_menu_output` is called

Reported by: darkskipper's profile darkskipper Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.8.3
Component: Administration Keywords: has-patch
Focuses: administration Cc:

Description

When plugins create certain kinds of administration menus, PHP warnings can occur when those menus are rendered. Although it's the plugin that generates the menu, it's the WordPress core code that is responsible for the warnings.

There may be related situations under which PHP warnings may occur during administration menu rendering, but the ones I've observed are, in summary:

  • When the PHP setting open_basedir is used.
  • When PHP is running under a Windows operating system.
  • When a plugin generates an administration menu featuring an external URL.

Possible related issues are...

  • ticket:25048 -- file_exists check in menu-header.php causes issue with backend menu on an IIS server
  • ticket:0132 -- PHP Warning at menu-header.php line 118

Steps to Reproduce

  1. Preparation: (a) Make a website testing area running under a Windows operating system. I tested with Windows 7. (b) Have a web server installed. I tested with Apache 2.2. (c) Have MySQL installed and prepare necessary user accounts and databases. I used MySQL 5.6. (d) Have PHP installed and configured so that the open_basedir setting is not used (the test plugin will do that). I tested with PHP 7.1.5. Ensure PHP error logging is enabled.
  1. Install WordPress 4.8.3.
  2. Create a directory wp-content/mu-plugins.
  3. Put the file test.admin.menu.php (attached) into that directory.
  4. Log into WordPress as administrator.
  5. Go to the dashboard.
  6. Check that the "NaNoWriMo" menu appears.
  7. Click the "Info" sub-menu under "NaNoWriMo".

Expected Behaviour

The "National Novel Writing Month" information page should appear in the dashboard. Below the info, there should be only one error message displayed as "test error, please ignore".

Actual Behaviour

In addition, there is an "open_basedir" error message displayed. This message may also appear in the PHP error log.

The above happens when PHP runs under Windows. Under Linux, the behaviour is as expected, with no extra warning messages.

Impact

The administrative menus are created correctly and behave as expected, however, there are PHP warning messages generated when those menus are rendered by wp_menu_output() in wp-admin/menu-header.php.

The problem may seem relatively benign, but it could lead to bloated logs. It may also indicate more serious underlying issues.

Analysis

The issue occurs when an administration menu (or sub-menu) with an external URL is created. The WordPress core attempts to make a file system path out of this URL, then checks for its existence. For example:

{DOCUMENT_ROOT}/wp-content/plugins/https:/nanowrimo.org

While this kind of operation may not be inherently dangerous, and does not seem to cause warnings unless the open_basedir PHP setting is used, concatenating URLs to file system paths is not an algorithmically correct thing to do.

Attachments (1)

test.admin.menu.php (8.4 KB) - added by darkskipper 7 years ago.
Must-use plugin to demonstrate the issue.

Download all attachments as: .zip

Change History (7)

@darkskipper
7 years ago

Must-use plugin to demonstrate the issue.

#1 @dd32
7 years ago

I don't think linking to external resources has ever been "supported" by add_menu_page(), but it certainly does work (edit: by passing a URL to the $menu_slug param), if only because we don't explicitly prevent urls. Have you found any documentation referencing the support anywhere by any chance?

While we could scatter a few is_url() like function calls throughout the menu generation, I'm not sure it's worth actually supporting myself.

Last edited 7 years ago by dd32 (previous) (diff)

#2 @darkskipper
7 years ago

The official documentation does not explicitly forbid nor deny the use or URLs for add_menu_page(), although it does refer to that parameter as a "slug" rather than anything else.

However, this issue came to my attention because a very popular plugin, All In One SEO Pack, uses this technique to provide an "Upgrade to Pro" menu that links to the vendor's website. This means that the URL use case for add_menu_page() is already out in the field.

#3 @antonmo
13 months ago

file_exists is still (in wp 6.4.1) wrecking havoc testing file paths that include a url.

file_exists(): open_basedir restriction in effect. File(C:\.../wp-content/plugins/http://www.domain.com/wp-admin/edit.php) is not within the allowed path(s): (C:\inetpub\wwwroot;C:\inetpub\temp\php_temp) in C:\inetpub\wwwroot\...\wp-admin\menu-header.php on line 259
Last edited 13 months ago by antonmo (previous) (diff)

#4 @antonmo
13 months ago

I have fixed this issue.

The issue is created by this line on line 259 (wp 6.4.1)

&& file_exists( WP_PLUGIN_DIR . "/$sub_file" )

Resolution:
Add the following line in a line prior to that

&& (filter_var($sub_file, FILTER_VALIDATE_URL) == false)

This ticket was mentioned in PR #6329 on WordPress/wordpress-develop by @narenin.


8 months ago
#5

  • Keywords has-patch added

#6 @narenin
8 months ago

Hi @darkskipper ,

I have created PR to fix this.

PR: https://github.com/WordPress/wordpress-develop/pull/6329

Note: See TracTickets for help on using tickets.