framework icon indicating copy to clipboard operation
framework copied to clipboard

url()->previousPath() potentially returns url

Open garygreen opened this issue 2 months ago • 5 comments

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

--

garygreen avatar Oct 20 '25 16:10 garygreen

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.

macropay-solutions avatar Oct 20 '25 18:10 macropay-solutions

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

garygreen avatar Oct 20 '25 20:10 garygreen

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?

BinaryKitten avatar Oct 21 '25 07:10 BinaryKitten

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!

iz-ahmad avatar Oct 21 '25 18:10 iz-ahmad

@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.

macropay-solutions avatar Oct 27 '25 12:10 macropay-solutions