Make WordPress Core

Opened 10 years ago

Closed 9 years ago

#25767 closed defect (bug) (fixed)

get_home_path() fails if admin area is SSL but not the frontend

Reported by: greglone's profile GregLone Owned by: johnbillion's profile johnbillion
Milestone: 4.0 Priority: normal
Severity: normal Version: 2.8
Component: Filesystem API Keywords: has-patch needs-testing needs-unit-tests
Focuses: Cc:

Description

Hello.

In case you use SSL for the administration area and not the frontend, get_home_path() will fail to retrieve the home path.
In this case:
home url: http://example.com
site url: https://example.com

Current version:

function get_home_path() {
	$home = get_option( 'home' );
	$siteurl = get_option( 'siteurl' );
	if ( ! empty( $home ) && 0 !== strcasecmp( $home, $siteurl ) ) {
		$wp_path_rel_to_home = str_ireplace( $home, '', $siteurl ); /* $siteurl - $home */
		$pos = strripos( str_replace( '\\', '/', $_SERVER['SCRIPT_FILENAME'] ), trailingslashit( $wp_path_rel_to_home ) );
		$home_path = substr( $_SERVER['SCRIPT_FILENAME'], 0, $pos );
		$home_path = trailingslashit( $home_path );
	} else {
		$home_path = ABSPATH;
	}

	return str_replace( '\\', '/', $home_path );
}

All the strings comparison or replacement will fail when they shouldn't.

Possible patch, remove the protocol:

function get_home_path() {
	$home = get_option( 'home' );
	$siteurl = get_option( 'siteurl' );
	$home = substr( $home, (strpos($home, 'https://') === 0 ? 8 : 7) );
	$siteurl = substr( $siteurl, (strpos($siteurl, 'https://') === 0 ? 8 : 7) );
	if ( ! empty( $home ) && 0 !== strcasecmp( $home, $siteurl ) ) {
		...

Thanks

Attachments (3)

remove_scheme.diff (1.5 KB) - added by GregLone 9 years ago.
remove_scheme.2.diff (1.5 KB) - added by GregLone 9 years ago.
Added the braces on the if statement.
25767.diff (807 bytes) - added by johnbillion 9 years ago.

Download all attachments as: .zip

Change History (13)

#1 @mark-k
10 years ago

maybe it is good that this fails as this is a bad idea to have that kind of a configuration. you are sending the cookies encrypted when you are in the admin section but unencrypted when you are on the frontend. If part of the site is SSLed then once a user login all of the site should be SSLed for him.

Maybe the true bug here is that you can have a protocol mismatch between home and siteurl.

Version 0, edited 10 years ago by mark-k (next)

#2 @dd32
10 years ago

Maybe the true bug here is that you can have a protocol mismatch between home and siteurl.

WordPress actually has separate cookies for this, it has wp-admin only cookies, and a more generic logged in cookie for the front end.

This configuration is completely supported and should work, the fact it doesn't IS a bug.

#3 @nacin
10 years ago

  • Component changed from General to Filesystem
  • Keywords close added

#4 @nacin
10 years ago

  • Keywords needs-patch added; close removed
  • Milestone changed from Awaiting Review to Future Release

Sorry, I misread dd32's comment.

#5 @jeremyclarke
9 years ago

+1 we are another site where this problem is causing havoc.

We use FORCE_SSL_ADMIN to secure the admin (logins etc) but don't secure the frontend because it's a lot more complicated to get all the frontend assets as https and thus avoid mixed content errors. (just stating the obvious as to why this config is desirable). Thus for us a home_url without https and a site_url with it is vital.

The outcome of this bug is that the permalinks page generates errors and becomes completely non-functional. No amount of chmod 777 will make .htacess writeable, which in many cases would mean people rendered unable to resave their permalinks (we had broken permalinks for awhile while I tried to track this down, TERRIBLE).

This also indirectly affects the WP-Supercache plugin which loses it's mind in mod_rewrite mode because it can't find the .htaccess file to assess or update it. I'm sure there are other things it breaks as well.

#6 @carlalexander
9 years ago

Ran into this issue today. A better fix along the lines above to remove the scheme entirely.

$home = get_option( 'home' );
$home = str_replace(parse_url($home, PHP_URL_SCHEME), '', $home);
$siteurl = get_option( 'siteurl' );
$siteurl = str_replace(parse_url($siteurl, PHP_URL_SCHEME), '', $siteurl);

That said providing a function that compares the two urls without the scheme might be more useful than doing it in the function itself.

#7 @GregLone
9 years ago

  • Version changed from 3.7.1 to 2.8

Doing a remove_url_scheme() function is not a bad idea actually.
See my diff file for the patch and the new function.

@GregLone
9 years ago

Added the braces on the if statement.

#8 @GregLone
9 years ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Summary changed from get_home_path() fails if admin is SSL and not the frontend to get_home_path() fails if admin area is SSL but not the frontend

@johnbillion
9 years ago

#9 @johnbillion
9 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from Future Release to 4.0

25767.diff simply forces the http scheme onto both URLs so the string comparison/replacement will work regardless of scheme on either URL.

Moving to 4.0 for consideration as part of our SSL improvements.

#10 @johnbillion
9 years ago

  • Owner set to johnbillion
  • Resolution set to fixed
  • Status changed from new to closed

In 28893:

Normalise the schemes used in get_home_path() so it returns the correct path for sites using SSL in the admin area but not the front end. Fixes #25767. Props GregLone for the initial patch.

Note: See TracTickets for help on using tickets.