Project

General

Profile

Actions

Bug #100215

closed

Redirect by redirectMode "refererDomains" is broken

Added by Torben Hansen over 1 year ago. Updated 5 months ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
felogin
Target version:
-
Start date:
2023-03-18
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
11
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

The redirect by refererDomains is broken since 2019, because:

1) The first regex only checks for non-SSL domains - see: preg_match('#^http://([[:alnum:]._-]+)/#', $url, $match)
2) The second regex is wrong (exception: Unknown modifier '/') - see: preg_match('/(?:^|\\.)' . $domain . '$/', $redirectDomain)


Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #98372: Redirect ”Defined by Domain entries“ does not workClosedTorben Hansen2022-09-19

Actions
Actions #1

Updated by Torben Hansen over 1 year ago

  • Description updated (diff)
Actions #2

Updated by Torben Hansen over 1 year ago

  • Related to Bug #98372: Redirect ”Defined by Domain entries“ does not work added
Actions #3

Updated by Ayke Halder over 1 year ago

I can confirm this.

Environment:

  • TYPO3 multi-site instance - with domains active in current environment (e.g Development)
    • domain-a.ddev.site
    • domain-b.ddev.site
    • domain-aXddev.site (any character replaces the dot)

All cases are only redirecting in the process of a successfull login. So no open redirect here.

To reproduce:

  1. Always log out frontend user first.
  2. Open case URL.
  3. Login frontend user.
  4. Check the redirect result.

Issue A: Domain-filter only active for http:// and ignored for httpS://

Systems: TYPO3 10, 11 (12?)

Setting "domains" ( https://docs.typo3.org/c/typo3/cms-felogin/11.5/en-us/Configuration/Index.html#domains )
plugin.tx_felogin_login.settings.domains = domain-a.ddev.site

… used with redirectMode = refererDomains ( https://docs.typo3.org/c/typo3/cms-felogin/11.5/en-us/Configuration/Index.html#redirectmode )
plugin.tx_felogin_login.settings.redirectMode = refererDomains

Case A.1

Case A.1-http: same domain
URL: https://domain-a.ddev.site/login/?referer=http://domain-a.ddev.site/another-page/
Result after successful login: Redirect to http://domain-a.ddev.site/another-page/
--> works correctly.

Case A.1-https: same domain
URL: https://domain-a.ddev.site/login/?referer=https://domain-a.ddev.site/another-page/
Result after successful login: Redirect to https://domain-a.ddev.site/another-page/
--> works somehow correctly. - Actually the domain filter is inactive here, but no issue as the domain is okay in this case.

Case A.2

Case A.2-http: foreign domain
URL: https://domain-a.ddev.site/login/?referer=http://foreign-domain.org/
Result after successful login: No redirect - domain not in environment.
--> works correctly.

Case A.2-https: foreign domain
URL: https://domain-a.ddev.site/login/?referer=https://foreign-domain.org/
Result after successful login: No redirect - domain is not in environment.
--> works correctly.

For implementation of the check "url is in enivronment" see Issue B in this mail.

Case A.3

Case A.3-http: multi-site another domain
URL: https://domain-a.ddev.site/login/?referer=http://domain-b.ddev.site/
Result after successful login: No redirect - domain not in plugin.tx_felogin_login.settings.domains
--> works correctly.

Case A.3-https: multi-site another domain
URL: https://domain-a.ddev.site/login/?referer=https://domain-b.ddev.site/
Result after successful login: Redirects to https://domain-b.ddev.site/
--> failure!

Wrong check in felogin/Classes/Redirect/RedirectModeHandler.php
https://github.com/TYPO3-CMS/felogin/blob/bd998d7b5c2f554591daf3b83a9fe82b6eca2458/Classes/Redirect/RedirectModeHandler.php#L128

Is:
if (preg_match('#^http://([[:alnum:]._-]+)/#', $url, $match)) {

Must be:
if (preg_match('#^https?://([[:alnum:]._-]+)/#', $url, $match)) {

Better be:
if (preg_match('#^https?://([[:alnum:].-]+)/#', $url, $match)) {

– There are no underscore characters in a hostname - only in domain names underscores are allowed. –

Case A.4

Case A.4-http: multi-site another domain with character/dash in place of dot
URL: https://domain-a.ddev.site/login/?referer=http://domain-aXddev.site/
Result after successful login: Redirects to http://domain-aXddev.site/
--> failure!

Case A.4-https: multi-site another domain with character/dash in place of dot
URL: https://domain-a.ddev.site/login/?referer=https://domain-aXddev.site/
Result after successful login: Redirects to https://domain-aXddev.site/
--> failure!

Wrong regex construction in felogin/Classes/Redirect/RedirectModeHandler.php
https://github.com/TYPO3-CMS/felogin/blob/bd998d7b5c2f554591daf3b83a9fe82b6eca2458/Classes/Redirect/RedirectModeHandler.php#L132

With:
$domain = "domain-a.ddev.site"
$redirectDomain = "domain-aXddev.site"

Is:
if (preg_match('/(?:^|\\.)' . $domain . '$/', $redirectDomain)) {

Should be:
if (preg_match('/(?:^|\\.)' . preg_quote($domain, '/') . '$/', $redirectDomain)) {

Actions #4

Updated by Torben Hansen over 1 year ago

The whole referer/refererDomains redirect stuff in ext:felogin is IMO way too complicated/error prone and should be simplified. Since the feature is broken since 2019 and nobody ever noticed, I suggest to completely remove it in v12, where we have BeforeRedirectEvent PSR-14 event to modify the final redirect as required.

Actions #5

Updated by Gerrit Code Review over 1 year ago

  • Status changed from New to Under Review

Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/78883

Actions #6

Updated by Gerrit Code Review over 1 year ago

Patch set 2 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/78883

Actions #7

Updated by Gerrit Code Review over 1 year ago

Patch set 3 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/78883

Actions #8

Updated by Gerrit Code Review over 1 year ago

Patch set 4 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/78883

Actions #9

Updated by Gerrit Code Review over 1 year ago

Patch set 5 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/78883

Actions #10

Updated by Gerrit Code Review over 1 year ago

Patch set 1 for branch 12.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/80071

Actions #11

Updated by Ayke Halder over 1 year ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #12

Updated by Gerrit Code Review over 1 year ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch 11.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/80132

Actions #13

Updated by Ayke Halder over 1 year ago

  • Status changed from Under Review to Resolved
Actions #14

Updated by Benni Mack 5 months ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF