Make WordPress Core

Opened 15 years ago

Closed 15 years ago

Last modified 15 years ago

#11032 closed defect (bug) (fixed)

Theme editor is not accessible

Reported by: pavelevap's profile pavelevap Owned by: westi's profile westi
Milestone: 2.9 Priority: normal
Severity: major Version: 2.9
Component: Themes Keywords: dev-feedback
Focuses: Cc:

Description

Hi, when I try to edit files in theme editor, there is only following error message "Sorry, cant call files with their real path". I updated to latest changeset, but no change.

Attachments (1)

11032.diff (3.4 KB) - added by dd32 15 years ago.

Download all attachments as: .zip

Change History (20)

#1 @dwright
15 years ago

  • Cc david_v_wright@… added

what is one of your file names?

(if you mouse over one of the file names on the right of the editor [Select theme to edit:, Theme Files, Templates,] what does the file param show?)

e.g.

?file=/themes/default/footer.php

does your file name contain a ':' after the first character? are you on windows? IIS? XAMPP?

can you list the steps needed to recreate this issue.

#2 @pavelevap
15 years ago

It is not possible to access editor for me...

http://localhost.cz/Wordpress/wp-admin/theme-editor.php?file=/themes/default/footer.php
  • "Sorry, that file cannot be edited."
http://localhost.cz/Wordpress/wp-admin/theme-editor.php
  • "Sorry, can’t call files with their real path."

I tried latest changeset from trunk, I can edit plugins without problems...

#3 @westi
15 years ago

  • Owner set to westi
  • Status changed from new to accepted

Does [12185] help?

#4 @pavelevap
15 years ago

No, no change...

#5 @pavelevap
15 years ago

These are my Apache logs:

127.0.0.1 - - [13/Nov/2009:20:05:23 +0100] "GET /Wordpress/wp-admin/theme-editor.php HTTP/1.1" 500 1214
127.0.0.1 - - [13/Nov/2009:20:05:24 +0100] "GET /Wordpress/wp-admin/css/install.css HTTP/1.1" 304 -
127.0.0.1 - - [13/Nov/2009:20:05:30 +0100] "GET /Wordpress/wp-admin/theme-editor.php?file=/themes/default/footer.php HTTP/1.1" 500 1192

#6 @westi
15 years ago

  • Keywords reporter-feedback added

What version of WordPress are you trying this with?

That doesn't look like 2.9-rare requests to me.

#7 @pavelevap
15 years ago

Yes, it is 2.9-rare. What is the problem? I use special Wordpress directory for SVN access to trunk. I updated a while before and latest changeset was successfully downloaded...

#8 @westi
15 years ago

Ok that is strange because current trunk theme editor requests look like this:

http://localhost/wordpress-trunk/wp-admin/theme-editor.php?file=/themes/default/404.php&theme=WordPress+Default&dir=theme

So your urls don't look as I expected.

#9 @pavelevap
15 years ago

Sorry, I could not get into theme editor, so I manually wrote supposed url for footer.php (theme-editor.php?file=/themes/default/footer.php) to test it.

You are right, now Apache shows following:

127.0.0.1 - - [14/Nov/2009:00:15:00 +0100] "GET /Wordpress/wp-admin/theme-editor.php HTTP/1.1" 500 1214
127.0.0.1 - - [14/Nov/2009:00:15:39 +0100] "GET /Wordpress/wp-admin/theme-editor.php?file=/themes/default/404.php&theme=WordPress+Default&dir=theme HTTP/1.1" 500 1214

#10 @janeforshort
15 years ago

  • Milestone changed from 2.9 to Future Release

Punting as no one else reporting this issue.

#11 follow-up: @toemon
15 years ago

I'm sorry by poor English.

I tested with XAMPP on Windows OS.

The value of $file and $allowed_files is the full paths on WP2.9. in theme-editor.php

$file = C:\xampp\htdocs\wordpress/wp-content/themes/default/rtl.css
$allowed_files[0] = C:\xampp\htdocs\wordpress/wp-content/themes/default/rtl.css
$allowed_files[1] = C:\xampp\htdocs\wordpress/wp-content/themes/default/style.css
     .
     .
$allowed_files[17] = C:\xampp\htdocs\wordpress/wp-content/themes/default/images/header-img.php

In wp-admin/theme-editor.php

$allowed_files = array_merge($themes[$theme]['Stylesheet Files'], $themes[$theme]['Template Files']);

if (empty($file)) {
	$file = $allowed_files[0];
} else {
	if ( 'theme' == $dir ) {
		$file = dirname(dirname($themes[$theme]['Template Dir'])) . $file ; 
	} else if ( 'style' == $dir) {
		$file = dirname(dirname($themes[$theme]['Stylesheet Dir'])) . $file ; 
	}
}

$real_file = validate_file_to_edit($file, $allowed_files);

stripslashes() removes '\'.

$file= C:xampphtdocswordpress/wp-content/themes/default/rtl.css 

And, validate_file() returns value 1.
When '\' is not removed.
Validate_file() returns value 2 because there is character ';'

In wp-admin/include/file.php

function validate_file_to_edit( $file, $allowed_files = '' ) {
	$file = stripslashes( $file );

	$code = validate_file( $file, $allowed_files );

	if (!$code )
		return $file;

	switch ( $code ) {
		case 1 :
			wp_die( __('Sorry, can’t edit files with “..” in the name. If you are trying to edit a file in your WordPress home directory, you can just type the name of the file in.' ));

		case 2 :
			wp_die( __('Sorry, can’t call files with their real path.' ));

		case 3 :
			wp_die( __('Sorry, that file cannot be edited.' ));
	}
}

In wp-includes/functions.php

function validate_file( $file, $allowed_files = '' ) {
	if ( false !== strpos( $file, '..' ))
		return 1;

	if ( false !== strpos( $file, './' ))
		return 1;

	if (':' == substr( $file, 1, 1 ))
		return 2;

	if (!empty ( $allowed_files ) && (!in_array( $file, $allowed_files ) ) )
		return 3;

	return 0;
}

#12 in reply to: ↑ 11 @toemon
15 years ago

Replying to toemon:

And, validate_file() returns value 1.

Correction:
And, validate_file() returns value 3.

#13 @pavelevap
15 years ago

  • Milestone changed from Future Release to 2.9

Yes, I also use XAMPP on Windows and I think many other users too. Is it possile to fix it in upcoming release?

#14 @dd32
15 years ago

since validate_file_to_edit expects a slashed file, $file = $allowed_files[0]; should probably be slashed to match what would be in the POST data.

Due to the changes which have been made relating to theme paths, and everything being absolute now, Perhaps branch 2 should be commented out of validate_file_to_edit (Leaving validate_file alone as its used elsewhere)

#15 @dd32
15 years ago

  • Keywords dev-feedback added; reporter-feedback removed

#16 @dd32
15 years ago

  • Severity changed from normal to major

upping severity as Theme editor component is broken on win32 systems.

#17 @dd32
15 years ago

not only that, but $real_file seems useless, as validate_file_to_edit() no longer returns anything, so its NULL on all platforms..

Attached patch appears to fix things for me.. but no idea of the repurcussions it may have.

Order in validate_file changed to increase security of theme edits while branch 2 is commented out (Else if it hit that condition, it'd pass right through without checking the allowed files)

@dd32
15 years ago

#18 @ryan
15 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [12310]) Fix file validation in theme editor. Props dd32. fixes #11032

#19 @ryan
15 years ago

Modified dd32's patch slightly to remove the stripslashes() from validate_file_to_edit() to eliminate confusion. Updating slashing in theme and plugin editors to be consistent.

Note: See TracTickets for help on using tickets.