[12.x] Fix security vulnerability in previousPath() method, preventing from returning external URLs

[ahmad-cit22]

This PR improves and fixes the previousPath() method in UrlGenerator with better security and origin validation, which solves the issue #57456.

Problem Summary

The previousPath() method is expected to return only the path of the previous URL (the doc SS attached for clarity). However, the existing implementation in previousPath() method had a security vulnerability where it returned a full external URL, when a malicious referrer header is sent.
This happened because it extracted the path from the previous URL without validating that the request originated from the same origin or not and the path extracting logic only worked for the same origin requests. So, this was making applications vulnerable to security issues including:

• Open redirect attacks via external domains in the referrer header
• XSS attacks through dangerous URI schemes (javascript:, data: etc)
• Protocol confusion attacks using different schemes (http vs https)

Solution

So, I solved this with -

1. Origin Validation: Added error-proof same-origin checking in previousPath(), that validates Host matching (case-insensitive), Scheme consistency (http/https) and also Port validation.
2. Dangerous URL Detection: Also implemented protection against malicious URI schemes.
3. Used built-in PHP function (parse_url()) for reliability
4. Fallback Handling: Enhanced fallback behavior for invalid or malicious referrers ensuring a good DX overall.

New Changes Benefits

• Prevents open redirect vulnerabilities by blocking external domain referrers
• Eliminates XSS or port-based attack with better URI detection and validation
• Handles different origin requests gracefully by returning fallback/root path
• Maintains backward compatibility while adding necessary fixes with security layers
• Provides fallback accurately for edge cases and malformed URL

Test Coverage

Added comprehensive tests covering:

• Same-origin URL allowing and External domain blocking
• Dangerous or suspicious URI scheme detect, Port-based attack prevention
• Protocol confusion prevention and Empty or null referrer handling
• Case-insensitive host validation
• Complex query strings and URL encoding
• Deep nested paths and trailing slash normalization
• Query string, fragment and special characters handling
• Fallback behavior verification and Path extraction accuracy

The changes made are fully backward compatible - existing functionality remains unchanged for legitimate same-origin requests while fixing the logic to match the expected behavior of the previousPath() method and adding security protection that was needed.
Even if a malicious referrer header is sent, it will return the fallback path or root path (e.g., /) instead of that external URL and thus handling this case silently and gracefully.

Note: Why I think this change should be added into 12.x? because it's a security vulnerability, not a feature. - The current behavior is not what is expected (the documentation also states that). Anyone relying on it is unknowingly vulnerable. The method name is previousPath() not previousUrlOrPath().. so, current behavior also violates the naming contract.

So this is surely a correctness fix, not any new feature or bringing breaking change. What we can do is -

• Clear change-log entry, marking it as as a security fix
• Note in upgrade guide with necessary info about the change and how it works.

@taylorotwell, Kindly let me know if we want to add/update anything in the documentation related to this change. Or if you have any disagreement on my thoughts, please inform me what can be improved. Thank you.

[mdzahid-pro]

Great work on this fix!

Validating the Referer origin and restricting previousPath() to same domain URLs is an important security improvement

This change effectively closes potential open redirect and Malicious scheme vulnerabilities while keeping the behavior consistent for legitimate use cases.

Thanks for prioritizing security and clarity here!

[garygreen]

Thank you for this! previous() can generate non app.url/origin of the app or unsafe style urls. Maybe that also needs to be behind the origin check. More generally pretty much all functions of the generator can return unsafe urls.

There is a isValidUrl function which to uses, so I'm wondering if somewhere along that chain that's the most logical place to put this additional safety check, or complementary isSafeUrl and call it during to function? Could be a opt in/out (on by default?), via a $safe variable and function to configure.

[macropay-solutions]

@garygreen all the places that refer to the referrer should be patched then, no? So just previous and previousPath

But maybe only previous should be changed and previousPath shouldn't? Or maybe previous should be called from previousPath..

[ahmad-cit22]

There is a isValidUrl function which to uses, so I'm wondering if somewhere along that chain that's the most logical place to put this additional safety check, or complementary isSafeUrl and call it during to function? Could be a opt in/out (on by default?), via a $safe variable and function to configure.

@garygreen Thanks for supporting! And yes, it can be a great enhancement. I also thought of this fact but still kept my changes limited within this issue only for now.. but I can extend this validation check for other parts as well as per suitability further and that can handle things in a more cleaner way.

But maybe only previous should be changed and previousPath shouldn't? Or maybe previous should be called from previousPath..

@macropay-solutions If we put the isSameOrigin and isDangerousUrl check inside the previous method, then there will be no more need of checking it inside previousPath again, instead we can just re-use previous method there. thus making the flow much cleaner. but that said, there will be still some things to be concerned of, so that it doesn't bring any breaking change. We can extend this right now if needed, or go with just the previousPath() change for the time being and bring the further changes later on.

Let's see what Taylor thinks of this.

[macropay-solutions]

@ahmad-cit22 have a look at this

[taylorotwell]

I'm not sure this is actually a security vulnerability? At the very least it's a breaking change as I'm not sure if people could be relying on this behavior.

[ahmad-cit22]

I'm not sure this is actually a security vulnerability?

Returning the whole url of external (non-origin) source when the method is expected to return only the path - isn’t really a security vulnerability issue?!

And talking about breaking change.. This is a concerning issue I agree. Then I will consider rePR with a more comprehensive solution regarding this to the master for the next major release. Hope that will make sense. Kindly let me know if you have any suggestion about it @taylorotwell

[macropay-solutions]

@garygreen @ahmad-cit22 Thank you. We patched it for us. And like we said in the beginning. It is the developer's job to check the referer if it is valid or not.