Ensure redirects from logme method are only done to trusted hosts (#17661)

* Ensure redirects from logme method are only done to trusted hosts

* add changelog

* sanitize host in exception message

3 files changed, 35 insertions, 0 deletions

diff --git a/plugins/Login/Controller.php b/plugins/Login/Controller.php
index eaa6c093c5..6b33afb7e8 100644
--- a/plugins/Login/Controller.php
+++ b/plugins/Login/Controller.php
@@ -311,6 +311,15 @@ class Controller extends \Piwik\Plugin\ControllerAdmin
         // remove password reset entry if it exists
         $this->passwordResetter->removePasswordResetInfo($login);
 
+        $parsedUrl = parse_url($urlToRedirect);
+
+        // only use redirect url if host is trusted
+        if (!empty($parsedUrl['host']) && !Url::isValidHost($parsedUrl['host'])) {
+            $e = new \Piwik\Exception\Exception('The redirect URL host is not valid, it is not a trusted host. If this URL is trusted, you can allow this in your config.ini.php file by adding the line <i>trusted_hosts[] = "'.Common::sanitizeInputValue($parsedUrl['host']).'"</i> under <i>[General]</i>');
+            $e->setIsHtmlMessage();
+            throw $e;
+        }
+
         if (empty($urlToRedirect)) {
             $redirect = Common::unsanitizeInputValue(Common::getRequestVar('form_redirect', false));
             $redirectParams = UrlHelper::getArrayFromQueryString(UrlHelper::getQueryFromUrl($redirect));
diff --git a/plugins/Login/tests/UI/Login_spec.js b/plugins/Login/tests/UI/Login_spec.js
index 0825646f8b..4bc3ea6e0b 100644
--- a/plugins/Login/tests/UI/Login_spec.js
+++ b/plugins/Login/tests/UI/Login_spec.js
@@ -34,6 +34,7 @@ describe("Login", function () {
         delete testEnvironment.bruteForceBlockIps;
         delete testEnvironment.bruteForceBlockThisIp;
         delete testEnvironment.queryParamOverride;
+        delete testEnvironment.configOverride.General;
         testEnvironment.save();
 
         await page.clearCookies();
@@ -44,6 +45,7 @@ describe("Login", function () {
         delete testEnvironment.bruteForceBlockIps;
         delete testEnvironment.bruteForceBlockThisIp;
         delete testEnvironment.queryParamOverride;
+        delete testEnvironment.configOverride.General;
         testEnvironment.save();
     });
 
@@ -255,4 +257,25 @@ describe("Login", function () {
 
         expect(await page.screenshot({ fullPage: true })).to.matchImage('bruteforcelog_blockedlogme');
     });
+
+    it("should show invalid host warning if redirect url is not trusted in logme", async function () {
+        testEnvironment.testUseMockAuth = 0;
+        testEnvironment.save();
+
+        await page.goto(formlessLoginUrl + "&url="+encodeURIComponent("https://www.matomo.org/security"));
+
+        expect(await page.screenshot({ fullPage: true })).to.matchImage('logme_redirect_invalid');
+    });
+
+    it("should redirect if host is trusted in logme", async function () {
+        testEnvironment.testUseMockAuth = 0;
+        testEnvironment.configOverride.General = {
+            "trusted_hosts": ["matomo.org"]
+        };
+        testEnvironment.save();
+
+        await page.goto(formlessLoginUrl + "&url="+encodeURIComponent("https://matomo.org/security/"));
+
+        expect(await page.getWholeCurrentUrl()).to.equal("https://matomo.org/security/");
+    });
 });
\ No newline at end of file
diff --git a/plugins/Login/tests/UI/expected-screenshots/Login_logme_redirect_invalid.png b/plugins/Login/tests/UI/expected-screenshots/Login_logme_redirect_invalid.png
new file mode 100644
index 0000000000..3b8b2359ea
--- /dev/null
+++ b/plugins/Login/tests/UI/expected-screenshots/Login_logme_redirect_invalid.png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:882e9ee5db5d65cba04c14b34f56058625c631df67d45b1dce3e46ac0231283b
+size 62022