diff --git a/src/Control/SAMLController.php b/src/Control/SAMLController.php index 85ddcc9..61b8160 100644 --- a/src/Control/SAMLController.php +++ b/src/Control/SAMLController.php @@ -262,6 +262,14 @@ protected function getRedirect() return $this->redirect($this->getRequest()->getSession()->get('BackURL')); } + // In SAMLHelper, we pass the BackURL to be returned to us in RelayState. + // If only assertion is signed, RelayState cannot be trusted. Prevent open relay + // as in // https://github.com/SAML-Toolkits/php-saml#avoiding-open-redirect-attacks + $relayState = $this->owner->getRequest()->postVar('RelayState'); + if ($relayState && Director::is_site_url($relayState)) { + $this->redirect($relayState); + } + // Spoofing attack, redirect to homepage instead of spoofing url if ($back && !Director::is_site_url($back)) { return $this->redirect(Director::absoluteBaseURL()); diff --git a/src/Helpers/SAMLHelper.php b/src/Helpers/SAMLHelper.php index 6993825..3fe6d18 100644 --- a/src/Helpers/SAMLHelper.php +++ b/src/Helpers/SAMLHelper.php @@ -75,7 +75,7 @@ public function redirect(RequestHandler $requestHandler = null, HTTPRequest $req $additionalGetQueryParams = $this->getAdditionalGETQueryParameters(); try { - $auth->login(Director::absoluteBaseURL() . 'saml/', $additionalGetQueryParams); + $auth->login($backURL, $additionalGetQueryParams); } catch (Exception $e) { /** @var LoggerInterface $logger */ $logger = Injector::inst()->get(LoggerInterface::class);