Fix unsecure redirect code.

This commit is contained in:
James Cole 2021-10-02 13:28:56 +02:00
parent 4e84a5c40c
commit 8662dfa4c0
No known key found for this signature in database
GPG Key ID: BDE6667570EADBD5
3 changed files with 58 additions and 2 deletions

View File

@ -29,8 +29,11 @@ namespace FireflyIII\Exceptions;
use ErrorException;
use FireflyIII\Jobs\MailError;
use Illuminate\Auth\AuthenticationException;
use Illuminate\Contracts\Foundation\Application;
use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
use Illuminate\Routing\Redirector;
use Illuminate\Session\TokenMismatchException;
use Illuminate\Support\Arr;
use Illuminate\Validation\ValidationException as LaravelValidationException;
@ -187,4 +190,58 @@ class Handler extends ExceptionHandler
)
);
}
/**
* Convert a validation exception into a response.
*
* @param Request $request
* @param LaravelValidationException $exception
*
* @return Application|RedirectResponse|Redirector
*/
protected function invalid($request, LaravelValidationException $exception): Application|RedirectResponse|Redirector
{
// protect against open redirect when submitting invalid forms.
$previous = $this->getPreviousUrl();
$redirect = $this->getRedirectUrl($exception);
return redirect($redirect ?? $previous)
->withInput(Arr::except($request->input(), $this->dontFlash))
->withErrors($exception->errors(), $request->input('_error_bag', $exception->errorBag));
}
/**
* Only return the previousUrl() if it is a valid URL. Return default redirect otherwise.
*
* @return string
*/
private function getPreviousUrl(): string
{
$safe = route('index');
$previous = url()->previous();
$previousHost = parse_url($previous, PHP_URL_HOST);
$safeHost = parse_url($safe, PHP_URL_HOST);
return null !== $previousHost && $previousHost === $safeHost ? $previous : $safe;
}
/**
* Only return the redirectTo property from the exception if it is a valid URL. Return NULL otherwise.
*
* @param LaravelValidationException $exception
*
* @return string|null
*/
private function getRedirectUrl(LaravelValidationException $exception): ?string
{
if (null === $exception->redirectTo) {
return null;
}
$safe = route('index');
$previous = $exception->redirectTo;
$previousHost = parse_url($previous, PHP_URL_HOST);
$safeHost = parse_url($safe, PHP_URL_HOST);
return null !== $previousHost && $previousHost === $safeHost ? $previous : $safe;
}
}

View File

@ -45,7 +45,6 @@ class StartFireflySession extends StartSession
$url = $request->fullUrl();
$forbiddenWords = strpos($url, 'offline') || strpos($url, 'jscript') || strpos($url, 'delete') || strpos($url, '/login') || strpos($url, '/json') || strpos($url, 'serviceworker') || strpos($url, '/attachments/view');
// also stop remembering "delete" URL's.
if (false === $forbiddenWords
&& 'GET' === $request->method()
&& !$request->ajax()) {

View File

@ -183,7 +183,7 @@ trait UserNavigation
// get host of previous URL:
$previous = parse_url($return, PHP_URL_HOST);
if ($default === $previous && (null === $errors || (0 === $errors->count())) && !Str::contains($return, $forbidden)) {
if (null !== $previous && $default === $previous && (null === $errors || (0 === $errors->count())) && !Str::contains($return, $forbidden)) {
Log::debug(sprintf('Saving URL %s under key %s', $return, $identifier));
session()->put($identifier, $return);