url()->previousPath() potentially returns url
Laravel Version
12.x
PHP Version
8.2
Database Driver & Version
No response
Description
The url()->previousPath() function is intended to return the path of the previous url - this is derived from the referer header from the url()->previous() function. As the client sends the referer header, it is untrustworthy and it can be different from the apps url, which it is assumed to always be.
This can be simulated like so:
// assuming config('app.url') is `https://myapp.local/`
// client could technically send whatever referer
request()->headers->set('referer', 'http://www.google.com/blah');
// unexpected full url, not path
dd(url()->previousPath()); // "http://www.google.com/blah"
// expected: "/blah", or possibly exception?
This is possibly a security vulnerability if developers expect previousPath() to not only return paths, but also restrict it from returning if the url is different to the apps one.
Steps To Reproduce
--
Relying on user input in unsafe by definition. What you are asking is a feature. But the referrer could be an external url in a real world scenario so restricting that should be done by the developer not by the framework.
What you are asking is a feature.
It's a bug, because the function is expected to return a path, not a full url - and furthermore, certainly not a full url to an external site. That could be exploited. Of course relying on user input is not safe - but that's precisely why this function exists as well, because its purpose is to more safely extract part of the path, so if developers rely on that then that's already potentially broken by design. I think the implementation was an oversight by original PR https://github.com/laravel/framework/pull/41661 - I would suggest using parse_url instead of doing regex/replace. cc @miclaus
I would agree that it is a bug, based on the test added for the functionality.
@garygreen - would you be up to PRing a fix? or would you like some assistance?
What you are asking is a feature.
It's a bug, because the function is expected to return a path, not a full url...
I also agree. Certainly it can be defined as a bug. Maybe it got escaped somehow earlier..it can be small issue but should be one of our concern to solve. Coz developers can use previousPath() assuming it only returns a safe path within their application (which is natural) and that can lead to-
- Open redirect vulnerabilities
- Unintended redirect to external url
thanks to @garygreen for pointing it out. I'm submitting a PR shortly, for fixing this...if u haven’t already fixed it!
@kakrusliandika we proposed inspired by @ahmad-cit22 :
public function previous($fallback = false)
{
$fallback = (string)$fallback;
$referrer = (string)$this->request->headers->get('referer');
if ('' === $referrer) {
$referrer = (string)$this->getPreviousUrlFromSession();
}
if ('' === $referrer) {
return (string)$this->to('' === $fallback ? '/' : $fallback);
}
$previous = (string)$this->to($referrer);
return '' !== $previous ? $previous : (string)$this->to('' === $fallback ? '/' : $fallback);
}
/**
* Get the previous path for the request.
*
* @param mixed $fallback
* @return string
*/
public function previousPath($fallback = false)
{
if (!\is_string($path = \parse_url($this->previous($fallback), PHP_URL_PATH))) {
return '/';
}
return $path === '/' ? $path : \rtrim($path, '/');
}
And the developer must validate the previous url from the referer as that can be considered user imput.