Skip to content

Commit d074a81

Browse files
committed
Warn about Open Redirect and Reply attacks
1 parent f08bc1e commit d074a81

File tree

6 files changed

+48
-9
lines changed

6 files changed

+48
-9
lines changed

README.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,37 @@ environment is not secure and will be exposed to attacks.
178178

179179
In production also we highly recommended to register on the settings the IdP certificate instead of using the fingerprint method. The fingerprint, is a hash, so at the end is open to a collision attack that can end on a signature validation bypass. Other SAML toolkits deprecated that mechanism, we maintain it for compatibility and also to be used on test environment.
180180

181+
182+
### Avoiding Open Redirect attacks ###
183+
184+
Some implementations uses the RelayState parameter as a way to control the flow when SSO and SLO succeeded. So basically the
185+
user is redirected to the value of the RelayState.
186+
187+
If you are using Signature Validation on the HTTP-Redirect binding, you will have the RelayState value integrity covered, otherwise, and
188+
on HTTP-POST binding, you can't trust the RelayState so before
189+
executing the validation, you need to verify that its value belong
190+
a trusted and expected URL.
191+
192+
Read more about Open Redirect [CWE-601](https://cwe.mitre.org/data/definitions/601.html).
193+
194+
195+
### Avoiding Reply attacks ###
196+
197+
A reply attack is basically try to reuse an intercepted valid SAML Message in order to impersonate a SAML action (SSO or SLO).
198+
199+
SAML Messages have a limited timelife (NotBefore, NotOnOrAfter) that
200+
make harder this kind of attacks, but they are still possible.
201+
202+
In order to avoid them, the SP can keep a list of SAML Messages or Assertion IDs alredy valdidated and processed. Those values only need
203+
to be stored the amount of time of the SAML Message life time, so
204+
we don't need to store all processed message/assertion Ids, but the most recent ones.
205+
206+
The OneLogin_Saml2_Auth class contains the [getLastRequestID](https://github.com/onelogin/php-saml/blob/f08bc1e8321cc8b855481153a26b9ed57a5201c2/lib/Saml2/Auth.php#L623), [getLastMessageId](https://github.com/onelogin/php-saml/blob/f08bc1e8321cc8b855481153a26b9ed57a5201c2/lib/Saml2/Auth.php#L709) and [getLastAssertionId](https://github.com/onelogin/php-saml/blob/f08bc1e8321cc8b855481153a26b9ed57a5201c2/lib/Saml2/Auth.php#L717) methods to retrieve the IDs
207+
208+
Checking that the ID of the current Message/Assertion does not exists in the list of the ones already processed will prevent reply
209+
attacks.
210+
211+
181212
Getting started
182213
---------------
183214

@@ -791,6 +822,8 @@ $_SESSION['samlNameidSPNameQualifier'] = $auth->getNameIdSPNameQualifier();
791822
$_SESSION['samlSessionIndex'] = $auth->getSessionIndex();
792823

793824
if (isset($_POST['RelayState']) && OneLogin_Saml2_Utils::getSelfURL() != $_POST['RelayState']) {
825+
// To avoid 'Open Redirect' attacks, before execute the
826+
// redirection confirm the value of $_POST['RelayState'] is a // trusted URL.
794827
$auth->redirectTo($_POST['RelayState']);
795828
}
796829

@@ -1129,6 +1162,8 @@ if (isset($_GET['sso'])) { // SSO action. Will send an AuthNRequest to the I
11291162

11301163
$_SESSION['samlUserdata'] = $auth->getAttributes(); // Retrieves user data
11311164
if (isset($_POST['RelayState']) && OneLogin_Saml2_Utils::getSelfURL() != $_POST['RelayState']) {
1165+
// To avoid 'Open Redirect' attacks, before execute the
1166+
// redirection confirm the value of $_POST['RelayState'] is a // trusted URL.
11321167
$auth->redirectTo($_POST['RelayState']); // Redirect if there is a
11331168
} // relayState set
11341169
} else if (isset($_GET['sls'])) { // Single Logout Service

demo1/index.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@
7474
if (!empty($errors)) {
7575
echo '<p>',implode(', ', $errors),'</p>';
7676
if ($auth->getSettings()->isDebugActive()) {
77-
echo '<p>'.$auth->getLastErrorReason().'</p>';
77+
echo '<p>'.htmlentities($auth->getLastErrorReason()).'</p>';
7878
}
7979
}
8080

@@ -91,6 +91,8 @@
9191
$_SESSION['samlSessionIndex'] = $auth->getSessionIndex();
9292
unset($_SESSION['AuthNRequestID']);
9393
if (isset($_POST['RelayState']) && OneLogin_Saml2_Utils::getSelfURL() != $_POST['RelayState']) {
94+
// To avoid 'Open Redirect' attacks, before execute the
95+
// redirection confirm the value of $_POST['RelayState'] is a // trusted URL.
9496
$auth->redirectTo($_POST['RelayState']);
9597
}
9698
} else if (isset($_GET['sls'])) {
@@ -105,9 +107,9 @@
105107
if (empty($errors)) {
106108
echo '<p>Sucessfully logged out</p>';
107109
} else {
108-
echo '<p>', implode(', ', $errors), '</p>';
110+
echo '<p>', htmlentities(implode(', ', $errors)), '</p>';
109111
if ($auth->getSettings()->isDebugActive()) {
110-
echo '<p>'.$auth->getLastErrorReason().'</p>';
112+
echo '<p>'.htmlentities($auth->getLastErrorReason()).'</p>';
111113
}
112114
}
113115
}

demo2/consume.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
$samlSettings = new OneLogin_Saml2_Settings();
1515
$samlResponse = new OneLogin_Saml2_Response($samlSettings, $_POST['SAMLResponse']);
1616
if ($samlResponse->isValid()) {
17-
echo 'You are: ' . $samlResponse->getNameId() . '<br>';
17+
echo 'You are: ' . htmlentities($samlResponse->getNameId()) . '<br>';
1818
$attributes = $samlResponse->getAttributes();
1919
if (!empty($attributes)) {
2020
echo 'You have the following attributes:<br>';
@@ -35,5 +35,5 @@
3535
echo 'No SAML Response found in POST.';
3636
}
3737
} catch (Exception $e) {
38-
echo 'Invalid SAML Response: ' . $e->getMessage();
38+
echo 'Invalid SAML Response: ' . htmlentities($e->getMessage());
3939
}

demo2/index.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
}
4040
echo '</tbody></table>';
4141
if (!empty($_SESSION['IdPSessionIndex'])) {
42-
echo '<p>The SessionIndex of the IdP is: '.$_SESSION['IdPSessionIndex'].'</p>';
42+
echo '<p>The SessionIndex of the IdP is: '.htmlentities($_SESSION['IdPSessionIndex']).'</p>';
4343
}
4444
} else {
4545
echo "<p>You don't have any attribute</p>";

endpoints/acs.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
$errors = $auth->getErrors();
1616

1717
if (!empty($errors)) {
18-
echo '<p>', implode(', ', $errors), '</p>';
18+
echo '<p>', htmlentities(implode(', ', $errors)), '</p>';
1919
exit();
2020
}
2121

@@ -27,6 +27,8 @@
2727
$_SESSION['samlUserdata'] = $auth->getAttributes();
2828
$_SESSION['IdPSessionIndex'] = $auth->getSessionIndex();
2929
if (isset($_POST['RelayState']) && OneLogin_Saml2_Utils::getSelfURL() != $_POST['RelayState']) {
30+
// To avoid 'Open Redirect' attacks, before execute the
31+
// redirection confirm the value of $_POST['RelayState'] is a // trusted URL.
3032
$auth->redirectTo($_POST['RelayState']);
3133
}
3234

@@ -44,7 +46,7 @@
4446
}
4547
echo '</tbody></table>';
4648
if (!empty($_SESSION['IdPSessionIndex'])) {
47-
echo '<p>The SessionIndex of the IdP is: '.$_SESSION['IdPSessionIndex'].'</p>';
49+
echo '<p>The SessionIndex of the IdP is: '.htmlentities($_SESSION['IdPSessionIndex']).'</p>';
4850
}
4951
} else {
5052
echo _('Attributes not found');

endpoints/sls.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@
1616
if (empty($errors)) {
1717
echo 'Sucessfully logged out';
1818
} else {
19-
echo implode(', ', $errors);
19+
echo htmlentities(implode(', ', $errors));
2020
}

0 commit comments

Comments
 (0)