New wp_validate_redirect() removes domain in some circumstances.

[rconde]

Last change to wp_validate_redirect() (5.2.3) breaks the redirect in some cases. I've checked on other sites that runs linux and this doesn't happen. This is happening on XAMPP for Windows.

In my case everything was working fine until the 5.2.3 update and now I get:

​https://wp-login.php/?loggedout=true
​https://wp-login.php/?checkemail=confirm
​https://wp-login.php/?checkemail=registered

As you can see, the domain is missing.

I've found the code causing this:

​https://github.com/WordPress/WordPress/commit/c86ee39ff4c1a79b93c967eb88522f5c09614a28

Commenting this code everything works fine again.

I've done a clean install of WordPress 5.2.3 to check if it's something that I've modified on my end but on the clean install it's still happening.

So definitely a bug.

[whyisjake]

Hi there @rconde,

Do you have any other filters/actions/plugins that are adding to those actions?

[jmmathc]

Can confirm it happens on Windows when logging out. I'd say it's because dirname returns backslashes on Windows, and they're not stripped correctly.

I've temporarily patched my servers with this fix, on the line that left trims the slash from $path:

$location = '/' . ltrim( $path . '/', '/\\' ) . $location;

[rconde]

Replying to whyisjake:

Hi there @rconde,

Do you have any other filters/actions/plugins that are adding to those actions?

It was on a fresh install on Windows.

As @jmmathc as said, it's a bug on backslashes on Windows.

[peterwilsoncc]

#47995 was marked as a duplicate.

[x2l2]

Replying to jmmathc:

Can confirm it happens on Windows when logging out. I'd say it's because dirname returns backslashes on Windows, and they're not stripped correctly.

I've temporarily patched my servers with this fix, on the line that left trims the slash from $path:

$location = '/' . ltrim( $path . '/', '/\\' ) . $location;

I had the same redirect problem when try to login, but the server is not windows

I try this change in wp-inculdes/pluggable.php but doesnt works for me

[rconde]

Try commenting this code in 'wp-includes/pluggable.php' and see if this works for you:

​https://github.com/WordPress/WordPress/commit/c86ee39ff4c1a79b93c967eb88522f5c09614a28

This is equal to revert back to previous version of WP as this is the only change pluggable.php had on the latest update.

Also, it would be nice to know what system configuration do you have if it's not Windows.

[Mat Lipe]

Patch the \wp-includes\pluggable.php file

[Mat Lipe]

I have tested against various environments and was able to recreate the issue as well as fix it with this patch.

[davidbaumwald]

#48017 was marked as a duplicate.

[Sixes]

I am seeing the same issue on a Ubuntu server (18.04.3 LTS). The WordPress install is version 5.2.3 on a multi-site setup.

I have tried removing the section at ​https://github.com/WordPress/WordPress/commit/c86ee39ff4c1a79b93c967eb88522f5c09614a28 and this makes no difference. Clearly adding a backslash to the ltrim() statement also has no effect.

In any case, does ltrim() really take three parameters? According to the php manual:

 ltrim ( string $str [, string $character_mask ] ) : string

The only other odd thing about this setup is that ​Fear of Landing redirects through Cloudflare.com.

Any suggestions as to how to get round this issue? Currently none of my users can log in.

Edit: Having checked further, it seems that wp_validate_redirect() is not actually being called. Also this may be a different issue as the user is (sometimes) getting:

ERROR: Cookies are blocked or not supported by your browser. You must enable cookies to use WordPress.

[rconde]

Replying to Sixes:

I am seeing the same issue on a Ubuntu server (18.04.3 LTS). The WordPress install is version 5.2.3 on a multi-site setup.

I have tried removing the section at ​https://github.com/WordPress/WordPress/commit/c86ee39ff4c1a79b93c967eb88522f5c09614a28 and this makes no difference. Clearly adding a backslash to the ltrim() statement also has no effect.

In any case, does ltrim() really take three parameters? According to the php manual:

 ltrim ( string $str [, string $character_mask ] ) : string

The only other odd thing about this setup is that ​Fear of Landing redirects through Cloudflare.com.

Any suggestions as to how to get round this issue? Currently none of my users can log in.

Edit: Having checked further, it seems that wp_validate_redirect() is not actually being called. Also this may be a different issue as the user is (sometimes) getting:

ERROR: Cookies are blocked or not supported by your browser. You must enable cookies to use WordPress.

This seems unrelated to this ticket. Fortunately you have mentioned Cloudflare...

I can tell you what is happening:

wp-login.php creates php cookie 'wordpress_test_cookie' when you access this page.

So I guess that you have configured Cloudflare incorrectly, so when you access wp-login.php you are getting a cached page from Cloudflare, not from your server, hence your server is not creating any cookie because the request is not getting the page from your server but from cloudflare.

Try setting Cloudflare development mode ON and try to log in and see if the problem persists. Please set the development mode On, wait at least 1 minute, reload wp-login.php and try.

If this fixes the problem, its your Cloudflare config, not WordPress.

Then the fix for you is to create a page rule in Cloudflare under 'Page Rules' -> Create page rule -> in the url field insert ​https://fearoflanding.com/*.php* and then click add a setting and select 'Cache level' -> Bypass and save and deploy.

Hope this fixes your problem.

[Sixes]

Thank you so much for that response @rconde, it was very helpful.

However it doesn't seem to have fixed the problem. I have now turned on development mode and cleared the Cloudflare cache. I'm still getting the same issue.

Digging a little further using curl, I find that WordPress appears to be setting a slew of cookies on login but these all have the domain .blog.me.uk which is the "parent" domain for the Wordpress install.For example:

set-cookie: wordpress_test_cookie=WP+Cookie+check; path=/; domain=.blog.me.uk; secure

As I understand it, this cookie will not be sent back whilst accessing the ​https://fearoflanding.com domain. I assume it is the lack of this cookie (or one of the other cookies with the same domain) that is causing the issue.

Any more ideas? I really appreciate the help.

[Sixes]

Well, oddly just uncommenting the line define( 'SUNRISE', 'on' ); in wp-config.php solved the problem. The cookies now all have the correct domain.

I have no idea why.

Thanks again @rconde for the advice.

[SergeyBiryukov]

#48042 was marked as a duplicate.

[justinahinon]

Hi,

Want to point out that the same issue happen when trying to reset a site password after I've enter the username and click on Reset.

[cmagrin]

#48148 was marked as a duplicate.

[daxelrod]

Noting the original commit which caused this bug was to fix a security issue: ​https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-16220

It would be useful to know what circumstances could lead to an open redirect to ensure a patch for this bug doesn't cause a regression.

[peterwilsoncc]

The WP unit tests don't include a Windows machine. If someone could run the unit tests on a Windows machine (ie, not in the docker container provided) and report back, that would be most helpful.

In 47980.diff​

• switched to using wp_normalize_path() before left trimming the URL. This ensures the dirname() output is converted to / throughout and keeps path normalization DRY
• Added some unit tests for validating redirects without hostnames

Test results found here ​https://github.com/WordPress/wordpress-develop/pull/108/checks

Edit: The Requests library includes remove_dot_segments() which may prove helpful here too, @SergeyBiryukov I'd be interested in your thoughts here.

[SergeyBiryukov]

Replying to peterwilsoncc:

In 47980.diff​

• switched to using wp_normalize_path() before left trimming the URL. This ensures the dirname() output is converted to / throughout and keeps path normalization DRY
• Added some unit tests for validating redirects without hostnames

Looks correct, though the tests appear to pass for me on Windows 10 both before and after the patch.

I could not yet reproduce the original issue either, so it looks like not all Windows configurations are affected. It would be helpful to know the Windows version on the affected installs, the server software, and whether the login URLs were customized in some way.

I'll see if I can come up with tests that fail before the patch and pass after.

[rconde]

I'm the OP of this ticket to give you some insight.

I'm on Windows 10 pro.

Download XAMPP for windows, clean install of xampp, then clean install of WP. Nothing customized. That's all.

I've just tested with and without the patch just in case a Windows hotfix fixed this, and not: Without the patch the original issue persists.

Replying to SergeyBiryukov:

Replying to peterwilsoncc:

In 47980.diff​

• switched to using wp_normalize_path() before left trimming the URL. This ensures the dirname() output is converted to / throughout and keeps path normalization DRY
• Added some unit tests for validating redirects without hostnames

Looks correct, though the tests appear to pass for me on Windows 10 both before and after the patch.

I could not yet reproduce the original issue either, so it looks like not all Windows configurations are affected. It would be helpful to know the Windows version on the affected installs, the server software, and whether the login URLs were customized in some way.

I'll see if I can come up with tests that fail before the patch and pass after.

[SergeyBiryukov]

Replying to rconde:

Download XAMPP for windows, clean install of xampp, then clean install of WP. Nothing customized. That's all.

Thanks! I was able to reproduce with WordPress installed in a root directory (previously was testing with a subdirectory).

47890.2.diff​ includes a test case that fails before and passes after the patch. Should be good to go.

[SergeyBiryukov]

In 46472:

Formatting: In wp_validate_redirect(), normalize the path when validating the location for relative URLs, to account for Windows paths.

Props peterwilsoncc, rconde, jmmathc, mat-lipe, Sixes, justinahinon, cmagrin, daxelrod, SergeyBiryukov.
Fixes #47980.

[SergeyBiryukov]

Reopening for 5.2.4.

[SergeyBiryukov]

In 46473:

Formatting: In wp_validate_redirect(), normalize the path when validating the location for relative URLs, to account for Windows paths.

Props peterwilsoncc, rconde, jmmathc, mat-lipe, Sixes, justinahinon, cmagrin, daxelrod, SergeyBiryukov.
Merges [46472] to the 5.2 branch.
Fixes #47980.