WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 9 months ago

#16822 closed defect (bug) (fixed)

FORCE_SSL_LOGIN causes wp-login.php to have an incorrect https link

Reported by: dbvista Owned by: jakub.tyrcha
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.1
Component: Security Keywords:
Focuses: Cc:

Description

In this bug, the WordPress logo on the login screen incorrectly links to an https URL. It is easy to reproduce.

First, define('FORCE_SSL_LOGIN', true) in wp-config.php. Then make sure you are logged out of WordPress. (Note: I am running multisite - I don't know if this matters or not.)

  1. Visit /wp-login.php. Fill in WRONG credentials (misspell your password) and click the Submit button.
  2. wp-login.php redisplays as expected, this time with an https URL.
  3. The WordPress logo on the form now links to https://your.site.com. If you click it, you are visiting your site over SSL.

This should not happen. The WordPress logo (and any other links on the login page) should render http URLs.

Attachments (2)

16822.patch (2.0 KB) - added by jakub.tyrcha 3 years ago.
16822.diff (1.4 KB) - added by willnorris 9 months ago.

Download all attachments as: .zip

Change History (14)

comment:1 dbvista3 years ago

This is WordPress 3.1.

comment:2 jamk3 years ago

  • Cc jamk added
  • Version set to 3.1

The same problem (URLs pointing to back to the site are https instead of http) occurs with FORCE_SSL_ADMIN turned on. In my case (WP3.1 with multisites in subdomains) both links in the wp-login.php page link into https://mysite.com instead of http://mysite.com. By both link I mean the WordPress logo in the middle (which seems to always point to the root of my website aka "main site" instead of the subdirectory) and the link in the upper left corner inside the <p id="backtoblog"> tag.

Changes to wp-login.php should be made to check whether http or https should be used in these places:

http://core.trac.wordpress.org/browser/trunk/wp-login.php#L89
http://core.trac.wordpress.org/browser/trunk/wp-login.php#L137

OR the check should be used in the two functions used:

apply_filters('login_headerurl', network_home_url() );

bloginfo('url');

comment:3 jakub.tyrcha3 years ago

  • Cc jakub.tyrcha added
  • Keywords has-patch needs-testing added
  • Owner set to jakub.tyrcha
  • Status changed from new to accepted

jakub.tyrcha3 years ago

comment:4 nacin3 years ago

Do we actually need the force_ssl_admin() || force_ssl_login() check? I imagine ! is_ssl() is enough.

Actually, I think the patch doesn't work right. In this case, you are viewing the page over SSL, but want http links. So that needs to be is_ssl(), not !.

This makes me think further about set_url_scheme().

comment:5 jakub.tyrcha3 years ago

  • Keywords has-patch needs-testing removed

OK, from what I see is_ssl() is not what I thought it was. However, using is_ssl to determine whether the home page should be displayed as http or https is a bad idea - with FORCE_SSL_LOGIN or FORCE_SSL_ADMIN set to 'true' there may be a situation where wp-login is https and the home page is http - using is_ssl() as you suggest would result in getting https instead of http for the home page.

comment:6 follow-up: nacin3 years ago

I don't know if there's a good solution for this.

There's no easy way to know whether the frontend is forced to SSL. So we don't actually know how we should be displaying links to the homepage. This occurs in the admin too, I imagine? The Visit Site link, for example.

comment:7 helenyhou3 years ago

The Visit Site and View links have the right scheme somehow inside the admin when using SSL, at least as I've experienced it. So somehow it works correctly there and there must be a reason why.

comment:8 SergeyBiryukov3 years ago

Closed #17827 as a duplicate.

comment:9 in reply to: ↑ 6 willnorris9 months ago

Replying to nacin:

I don't know if there's a good solution for this.

There's no easy way to know whether the frontend is forced to SSL. So we don't actually know how we should be displaying links to the homepage. This occurs in the admin too, I imagine? The Visit Site link, for example.

Well, sure there is (sort of). get_home_url currently has:

    if ( is_ssl() && ! is_admin() )
      $scheme = 'https';
    else
      $scheme = parse_url( $url, PHP_URL_SCHEME );

The idea here being, if you're currently browsing with SSL, then we assume you want to continue doing so. The exception being if you're currently browsing wp-admin, in which case we don't make that assumption, since sites with FORCE_SSL_ADMIN will likely have secure admin pages, but not the frontend site. In that case, we just don't touch the scheme and use whatever we got back from get_option('home').

We simply need to extend that logic to cover wp-login.php as well. There is no equivalent is_login_page() function to know if code is running on the login page, so that would need to be added.

willnorris9 months ago

comment:10 willnorris9 months ago

patch added to add an is_login() function and use that inside of get_home_url() as I described above.

comment:11 nacin9 months ago

  • Milestone changed from Awaiting Review to 3.7

Per discussion in person, going to leave out is_login() for now — conversation for another ticket.

comment:12 nacin9 months ago

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

In 24844:

If wp-login.php is accessed over HTTPS, get_home_url() should not return HTTPS. This is the same assumption we use in the admin.

props willnorris.
fixes #16822.

Note: See TracTickets for help on using tickets.