Skip to content

Commit 110fc75

Browse files
committed
Warn about Open Redirect and Reply attacks
1 parent 107757b commit 110fc75

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
@@ -154,6 +154,37 @@ environment is not secure and will be exposed to attacks.
154154

155155
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.
156156

157+
158+
### Avoiding Open Redirect attacks ###
159+
160+
Some implementations uses the RelayState parameter as a way to control the flow when SSO and SLO succeeded. So basically the
161+
user is redirected to the value of the RelayState.
162+
163+
If you are using Signature Validation on the HTTP-Redirect binding, you will have the RelayState value integrity covered, otherwise, and
164+
on HTTP-POST binding, you can't trust the RelayState so before
165+
executing the validation, you need to verify that its value belong
166+
a trusted and expected URL.
167+
168+
Read more about Open Redirect [CWE-601](https://cwe.mitre.org/data/definitions/601.html).
169+
170+
171+
### Avoiding Reply attacks ###
172+
173+
A reply attack is basically try to reuse an intercepted valid SAML Message in order to impersonate a SAML action (SSO or SLO).
174+
175+
SAML Messages have a limited timelife (NotBefore, NotOnOrAfter) that
176+
make harder this kind of attacks, but they are still possible.
177+
178+
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
179+
to be stored the amount of time of the SAML Message life time, so
180+
we don't need to store all processed message/assertion Ids, but the most recent ones.
181+
182+
The OneLogin_Saml2_Auth class contains the [getLastRequestID](https://github.com/onelogin/php-saml/blob/107757b29402ec5b2525c3e37d58e3ed8ac56f6e/src/Saml2/Auth.php#L657), [getLastMessageId](https://github.com/onelogin/php-saml/blob/107757b29402ec5b2525c3e37d58e3ed8ac56f6e/src/Saml2/Auth.php#L793) and [getLastAssertionId](https://github.com/onelogin/php-saml/blob/107757b29402ec5b2525c3e37d58e3ed8ac56f6e/src/Saml2/Auth.php#L801) methods to retrieve the IDs
183+
184+
Checking that the ID of the current Message/Assertion does not exists in the list of the ones already processed will prevent reply
185+
attacks.
186+
187+
157188
Getting started
158189
---------------
159190

@@ -760,6 +791,8 @@ $_SESSION['samlNameidSPNameQualifier'] = $auth->getNameIdSPNameQualifier();
760791
$_SESSION['samlSessionIndex'] = $auth->getSessionIndex();
761792

762793
if (isset($_POST['RelayState']) && OneLogin\Saml2\Utils::getSelfURL() != $_POST['RelayState']) {
794+
// To avoid 'Open Redirect' attacks, before execute the
795+
// redirection confirm the value of $_POST['RelayState'] is a // trusted URL.
763796
$auth->redirectTo($_POST['RelayState']);
764797
}
765798

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

10991132
$_SESSION['samlUserdata'] = $auth->getAttributes(); // Retrieves user data
11001133
if (isset($_POST['RelayState']) && OneLogin\Saml2\Utils::getSelfURL() != $_POST['RelayState']) {
1134+
// To avoid 'Open Redirect' attacks, before execute the
1135+
// redirection confirm the value of $_POST['RelayState'] is a // trusted URL.
11011136
$auth->redirectTo($_POST['RelayState']); // Redirect if there is a
11021137
} // relayState set
11031138
} else if (isset($_GET['sls'])) { // Single Logout Service

demo1/index.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@
7676
$errors = $auth->getErrors();
7777

7878
if (!empty($errors)) {
79-
echo '<p>' . implode(', ', $errors) . '</p>';
79+
echo '<p>' . htmlentities(implode(', ', $errors)) . '</p>';
8080
if ($auth->getSettings()->isDebugActive()) {
81-
echo '<p>'.$auth->getLastErrorReason().'</p>';
81+
echo '<p>'.htmlentities($auth->getLastErrorReason()).'</p>';
8282
}
8383
}
8484

@@ -96,6 +96,8 @@
9696

9797
unset($_SESSION['AuthNRequestID']);
9898
if (isset($_POST['RelayState']) && Utils::getSelfURL() != $_POST['RelayState']) {
99+
// To avoid 'Open Redirect' attacks, before execute the
100+
// redirection confirm the value of $_POST['RelayState'] is a // trusted URL
99101
$auth->redirectTo($_POST['RelayState']);
100102
}
101103
} else if (isset($_GET['sls'])) {
@@ -110,9 +112,9 @@
110112
if (empty($errors)) {
111113
echo '<p>Sucessfully logged out</p>';
112114
} else {
113-
echo '<p>' . implode(', ', $errors) . '</p>';
115+
echo '<p>' . htmlentities(implode(', ', $errors)) . '</p>';
114116
if ($auth->getSettings()->isDebugActive()) {
115-
echo '<p>'.$auth->getLastErrorReason().'</p>';
117+
echo '<p>'.htmlentities($auth->getLastErrorReason()).'</p>';
116118
}
117119
}
118120
}

demo2/consume.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,5 +38,5 @@
3838
echo 'No SAML Response found in POST.';
3939
}
4040
} catch (Exception $e) {
41-
echo 'Invalid SAML Response: ' . $e->getMessage();
41+
echo htmlentities('Invalid SAML Response: ' . $e->getMessage());
4242
}

demo2/index.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
}
4444
echo '</tbody></table>';
4545
if (!empty($_SESSION['IdPSessionIndex'])) {
46-
echo '<p>The SessionIndex of the IdP is: '.$_SESSION['IdPSessionIndex'].'</p>';
46+
echo '<p>The SessionIndex of the IdP is: '.htmlentities($_SESSION['IdPSessionIndex']).'</p>';
4747
}
4848
} else {
4949
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
@@ -18,7 +18,7 @@
1818
$errors = $auth->getErrors();
1919

2020
if (!empty($errors)) {
21-
echo '<p>' . implode(', ', $errors) . '</p>';
21+
echo '<p>' . htmlentities(implode(', ', $errors)) . '</p>';
2222
exit();
2323
}
2424

@@ -30,6 +30,8 @@
3030
$_SESSION['samlUserdata'] = $auth->getAttributes();
3131
$_SESSION['IdPSessionIndex'] = $auth->getSessionIndex();
3232
if (isset($_POST['RelayState']) && Utils::getSelfURL() != $_POST['RelayState']) {
33+
// To avoid 'Open Redirect' attacks, before execute the
34+
// redirection confirm the value of $_POST['RelayState'] is a // trusted URL.
3335
$auth->redirectTo($_POST['RelayState']);
3436
}
3537

@@ -47,7 +49,7 @@
4749
}
4850
echo '</tbody></table>';
4951
if (!empty($_SESSION['IdPSessionIndex'])) {
50-
echo '<p>The SessionIndex of the IdP is: '.$_SESSION['IdPSessionIndex'].'</p>';
52+
echo '<p>The SessionIndex of the IdP is: '.htmlentities($_SESSION['IdPSessionIndex']).'</p>';
5153
}
5254
} else {
5355
echo _('Attributes not found');

endpoints/sls.php

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

0 commit comments

Comments
 (0)