From 158a21257f6a10f19e76b1edc0b55c2111b6a49a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20B=C3=BCrk?= <stefan@buerk.tech>
Date: Tue, 12 Sep 2023 14:34:22 +0200
Subject: [PATCH] [BUGFIX] Avoid redirect loop for empty redirect url
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Sending a redirect response with a empty `Location` is
invalid per RFC. Browser vendor are dealing differntly
with it.
* Firefox executes a redirect to the current url, leading
to an `endless` redirect chain - stopping it after some
recursions with a coresponding notice in the network tab.
* Chrome determines this and is doing nothing at all with
it - leading to a white page.
From the [1] RFC regarding invalid URI spec for `Location`:
> Note: Some recipients attempt to recover from Location
> fields that are not valid URI references. This
> specification does not mandate or define such
> processing, but does allow it for the sake of
> robustness.
A matching redirect record with a manually entered `/` as
redirect target leads in TYPO3 v11 to this behaviour. This
can be mitigated by selecting the corresponding site root.
For TYPO3 v12 and upwards a change in the LinkHandling has
been introduced which properly handles the `/` in the link
generation and correctly returning a `/` as redirect url.
That change has quite some impact and is not reasonable to
be backported to TYPO3 v11 within #100958.
This change adds an additionally guard to the `RedirectHandler`
to handle empty redirect urls as endless loop, just logging
it and not responding with an redirect. This helps in v11 and
keeps a safety guard for the future in this place.
[1] https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.2
Resolves: #100791
Related: #100958
Releases: main, 12.4, 11.5
Change-Id: I2af2d5bf759a277ade45bd0f7740ffe0099003b3
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/81280
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
---
.../Http/Middleware/RedirectHandler.php | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/typo3/sysext/redirects/Classes/Http/Middleware/RedirectHandler.php b/typo3/sysext/redirects/Classes/Http/Middleware/RedirectHandler.php
index 39cec49cf089..074e8f891d71 100644
--- a/typo3/sysext/redirects/Classes/Http/Middleware/RedirectHandler.php
+++ b/typo3/sysext/redirects/Classes/Http/Middleware/RedirectHandler.php
@@ -66,7 +66,11 @@ class RedirectHandler implements MiddlewareInterface, LoggerAwareInterface
$url = $this->redirectService->getTargetUrl($matchedRedirect, $request);
if ($url instanceof UriInterface) {
if ($this->redirectUriWillRedirectToCurrentUri($request, $url)) {
- if ($url->getFragment()) {
+ if ($this->isEmptyRedirectUri($url)) {
+ // Empty uri leads to a redirect loop in Firefox, whereas Chrome would stop it but not displaying anything.
+ // @see https://forge.typo3.org/issues/100791
+ $this->logger->error('Empty redirect points to itself! Aborting.', ['record' => $matchedRedirect, 'uri' => (string)$url]);
+ } elseif ($url->getFragment()) {
// Enrich error message for unsharp check with target url fragment.
$this->logger->error('Redirect ' . $url->getPath() . ' eventually points to itself! Target with fragment can not be checked and we take the safe check to avoid redirect loops. Aborting.', ['record' => $matchedRedirect, 'uri' => (string)$url]);
} else {
@@ -120,6 +124,9 @@ class RedirectHandler implements MiddlewareInterface, LoggerAwareInterface
*/
protected function redirectUriWillRedirectToCurrentUri(ServerRequestInterface $request, UriInterface $redirectUri): bool
{
+ if ($this->isEmptyRedirectUri($redirectUri)) {
+ return true;
+ }
$requestUri = $request->getUri();
$redirectIsAbsolute = $redirectUri->getHost() && $redirectUri->getScheme();
$requestUri = $this->sanitizeUriForComparison($requestUri, !$redirectIsAbsolute);
@@ -180,4 +187,13 @@ class RedirectHandler implements MiddlewareInterface, LoggerAwareInterface
return $uri;
}
+
+ /**
+ * Empty uri leads to a redirect loop in Firefox, whereas Chrome would stop it but not displaying anything.
+ * @see https://forge.typo3.org/issues/100791
+ */
+ private function isEmptyRedirectUri(UriInterface $uri): bool
+ {
+ return (string)$uri === '';
+ }
}
--
GitLab