[12.x] redirect response enforce same origin

[browner12]

it can be dangerous to allow user controlled data to determine a redirect target. this is something that could possibly be exploited when using something like the "referer" header to generate your target, like with our back() helper. this new enforceSameOrigin() method forces the target URL to match the origin (scheme, host, and port) of the current request URL.

you must provide a fallback (ideally an absolute URL) for if the check fails. optionally, you may disable scheme and/or port validation. you may not disable hostname validation, because honestly then what's the point?

ideally I would be able to use the $this->request property to directly access the scheme, host, and port, but because of how the Request object handles standard and non-standard ports differently than the Uri class, it's more reliable to turn them both into Uris so we get consistent behavior to compare.

this implementation allows us to build our redirects in a composable way. We can chain on this method to any redirect response. It would likely mostly be used in conjunction with the Redirector::back() method, due to its use of the "referer" header. However, maybe you're using a user generated database entry like Redirect::to($userDefinedTarget) that you also wish limit to same origin. This pattern gives full control.

return redirect() ->back() ->enforceSameOrigin(route('dashboard'));

This was driven by a recent security audit my company did on our website. One of the findings, although deemed minor, was our use of the "referer" header for redirects. Our solution at the time was to remove any back() calls in favor of explicit routes. This feature would allow us to maintain "back" functionality while still preventing the attack vector.

[jmarble]

only compare if target has host, scheme, or port.

[browner12]

I think any additional checks we would put prior to this would fail just as quickly as these conditionals will.

[axlon]

Love this (have also had this come out of security audits), but would personally prefer a flag on the back method rather than a separate method.

I think if this PR is merged a follow up should probably be made to make this opt-out rather than opt-in for Laravel 13

[rodrigopedra]

After noticing some previous attempts to change this behavior and after undergoing the same security recommendations, I ended up adding this middleware to my projects:

<?php namespace App\Http\Middleware; use Illuminate\Http\Request; use Illuminate\Support\Uri; use Symfony\Component\HttpFoundation\Response; class RemoveExternalReferrer { public function handle(Request $request, \Closure $next): Response { if (! $this->fromSameOrigin($request)) { $request->headers->remove('referer'); } return $next($request); } private function fromSameOrigin(Request $request): bool { if (\blank($referrer = $request->headers->get('referer'))) { return true; } $origin = Uri::of($referrer); $current = $request->uri(); return $origin->scheme() === $current->scheme() && $origin->host() === $current->host() && $origin->port() === $current->port(); } }

To be honest, I used only to check for matching the host. I added the checks for scheme and port after seeing this PR's changes. Thanks!

That way all the helpers and redirects that depend on the url()->previous() value work as expected.

---

EDIT

For reference, see the discussion on this issue: #14642

[browner12]

@axlon the idea would be to build this into back() by default in v13, and make it opt out. I want to keep this as a separate method for composability, since there are other possible scenarios when this would be valuable.

@rodrigopedra glad it helped! Also, very interesting discussion on that issue thread. Not sure if anything has changed since then, but seems like most people feel like it'd be a difficult (if not impossible) vector to exploit. Seeing as that's up for debate, and how this is a pretty simple opt-in fix for now, I hoping we can still merge it to put the possible concern to bed.

[onlime]

Thanks @browner12, great PR!

What's your recommendation for already baking this into (or overriding) the current back() helper as the default in v12.36.0, instead of waiting for v13? I'd also prefer to throw a brutal default exception or pass a custom exception (with a render() implementation for RedirectResponse) that gets reported, rather than providing a fallback route. Did you consider that?

[browner12]

For v12 the best way would probably be to chain it:

return redirect() ->back() ->enforceSameOrigin('/fallback');

I did consider ways to report this but didn't come up with anything I was happy with.

One thing you could do for the time being is set the fallback to a route that logs the occurrence, and then redirects to your actual fallback.