WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#11974 closed defect (bug) (fixed)

uninitialized variable causes "sorry, that file cannot be edited"

Reported by: cwgservices Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.9.1
Component: Administration Keywords: has-patch commit
Focuses: Cc:

Description

This bug is present in both wp-admin/theme-editor.php and wp-admin/plugin-editor in version 2.9.1 of wordpress. Depending on what other plugins happen to be activated, i.e. what they happen to leave on the stack, there are times when any attempt to use the theme editor or plugin editor produces the error message. Deactivating a recently added plugin restores normal operation, but the bug is in the core rather than the plugin.

The variable $file is not initialized yet it is tested; if it happens to be nonempty the error will occur. The error comes from the subsequent call to validate_file_for_edit().

I am attaching the patched files that I am using until the core gets officially fixed. The changes are surrounded with comments including my initials (cwg). There is one patched section in each of the two files.

Attachments (3)

theme-editor.php (8.8 KB) - added by cwgservices 6 years ago.
theme editor with patch for unitialized variable $file
plugin-editor.php (8.6 KB) - added by cwgservices 6 years ago.
plugin editor with patch for unitialized variable $file
wp_reset_vars.diff (622 bytes) - added by scribu 6 years ago.
more aggressive wp_reset_vars()

Download all attachments as: .zip

Change History (8)

@cwgservices6 years ago

theme editor with patch for unitialized variable $file

@cwgservices6 years ago

plugin editor with patch for unitialized variable $file

comment:1 follow-up: @scribu6 years ago

  • Keywords reporter-feedback added; editor theme-editor plugin-editor file cannot be edited removed

$file is initialized by wp_reset_vars(), which means it's set using the $_GET variables.

Could you provide the steps to reproduce your problem?

comment:2 @scribu6 years ago

  • Priority changed from high to normal

comment:3 in reply to: ↑ 1 @cwgservices6 years ago

Replying to scribu:

$file is initialized by wp_reset_vars(), which means it's set using the $_GET variables.

Could you provide the steps to reproduce your problem?

On our site we have the following plugins:
===============text copied from plugins page=================
Akismet

Akismet checks your comments against the Akismet web service to see if they look like spam or not. You need a WordPress.com API key to use it. You can review the spam it catches under "Comments." To show off your Akismet stats just put <?php akismet_counter(); ?> in your template. See also: WP Stats plugin.

Deactivate | Edit

Version 2.2.7 | By Matt Mullenweg | Visit plugin site
Bei Fen

A backup plugin for Wordpress. You can create full, files-only, and database-only backups! Scheduled backups are also possible!

Settings | Deactivate | Edit

Version 1.4.2 | By David Schneider | Visit plugin site
Contact Form 7

Just another contact form plugin. Simple but flexible.

Settings | Deactivate | Edit

Version 2.1 | By Takayuki Miyoshi | Visit plugin site
Inline Editor

Allows a logged in user to edit content on the displaying page instead of having to log in to the admin area. Utilises Brian Kirchoff's brilliant NicEdit component.

Settings | Deactivate | Edit

Version 0.7.1 | By WPXpand | Visit plugin site
TDO Mini Forms

This plugin allows you to add custom posting forms to your website that allows your readers (including non-registered) to submit posts.

Deactivate | Edit

Version 0.13.7 | By Mark Cunningham | Visit plugin site
User Role Subscriptions

This simple wordpress plugin is designed to manage user role subscriptions. You may charge differently for all roles and manage their subscription periods.

Deactivate | Edit

Version 1.5.0 | By Jonathon Byrd | Visit plugin site
WP-Cumulus

Flash based Tag Cloud for WordPress

Deactivate | Edit

Version 1.23 | By Roy Tanck | Visit plugin site
WP-reCAPTCHA

Integrates reCAPTCHA anti-spam solutions with wordpress

Deactivate | Edit

Version 2.9.6 | By Jorge Peña | Visit plugin site

===============end text copied from plugins page=================
Before I made the patch that was attached to the original post, activating the plugin User Role Subscriptions caused us to be unable to use the theme_editor or plugin_editor. Any time a link was clicked that would normally pull up one of those, we would get a fatal error "that file cannot be edited". I inserted some code to dump the incoming variables to the validate_file_for_edit() function, and discovered that when the plugin was activated any editor click produced an attempt to edit wp_load.php and therefore created the error condition. Looking at the calls to that function I noticed the [apparently] unitialized variable, and came up with my solution. Is there some way that the plugin or theme editors can be called with a filename plugged in to a $_GET variable? Are those some sort of global variables? Is that the purpose of this code? If so, how can I test to see where the obviously incorrect filename is being written to the variable.

Thank you. It is possible based on your previous response that the error is in the plugin, after all. But I have seen reports of similar behavior with a number of plugins (whose names I cannot remember) which implies either a problem in the two files I mentioned or a common misconception among those who write plugins...

@scribu6 years ago

more aggressive wp_reset_vars()

comment:4 @scribu6 years ago

  • Keywords has-patch commit added; reporter-feedback removed
  • Milestone changed from Unassigned to 3.0

For some reason, the plugin set the $file variable to ../wp-load.php

The problem came from the fact that wp_reset_vars() was checking if $file was previously set, instead of resetting the variable regardless, like its name suggests.

wp_reset_vars.diff fixes this.

comment:5 @scribu6 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.