Skip to content

Commit ca87a2a

Browse files
committed
Warn about Open Redirect and Reply attacks
1 parent b8214b7 commit ca87a2a

6 files changed

Lines changed: 49 additions & 11 deletions

File tree

README.md

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,37 @@ environment is not secure and will be exposed to attacks.
148148

149149
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.
150150

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

@@ -754,6 +785,8 @@ $_SESSION['samlNameidSPNameQualifier'] = $auth->getNameIdSPNameQualifier();
754785
$_SESSION['samlSessionIndex'] = $auth->getSessionIndex();
755786

756787
if (isset($_POST['RelayState']) && OneLogin\Saml2\Utils::getSelfURL() != $_POST['RelayState']) {
788+
// To avoid 'Open Redirect' attacks, before execute the
789+
// redirection confirm the value of $_POST['RelayState'] is a // trusted URL.
757790
$auth->redirectTo($_POST['RelayState']);
758791
}
759792

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

10931126
$_SESSION['samlUserdata'] = $auth->getAttributes(); // Retrieves user data
10941127
if (isset($_POST['RelayState']) && OneLogin\Saml2\Utils::getSelfURL() != $_POST['RelayState']) {
1128+
// To avoid 'Open Redirect' attacks, before execute the
1129+
// redirection confirm the value of $_POST['RelayState'] is a // trusted URL.
10951130
$auth->redirectTo($_POST['RelayState']); // Redirect if there is a
10961131
} // relayState set
10971132
} else if (isset($_GET['sls'])) { // Single Logout Service
@@ -1100,7 +1135,7 @@ if (isset($_GET['sso'])) { // SSO action. Will send an AuthNRequest to the I
11001135
if (empty($errors)) {
11011136
echo '<p>Sucessfully logged out</p>';
11021137
} else {
1103-
echo '<p>' . implode(', ', $errors) . '</p>';
1138+
echo '<p>' . htmlentities(implode(', ', $errors)) . '</p>';
11041139
}
11051140
}
11061141

demo1/index.php

Lines changed: 7 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,9 @@
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']
101+
// is a trusted URL.
99102
$auth->redirectTo($_POST['RelayState']);
100103
}
101104
} else if (isset($_GET['sls'])) {
@@ -110,9 +113,9 @@
110113
if (empty($errors)) {
111114
echo '<p>Sucessfully logged out</p>';
112115
} else {
113-
echo '<p>' . implode(', ', $errors) . '</p>';
116+
echo '<p>' . htmlentities(implode(', ', $errors)) . '</p>';
114117
if ($auth->getSettings()->isDebugActive()) {
115-
echo '<p>'.$auth->getLastErrorReason().'</p>';
118+
echo '<p>'.htmlentities($auth->getLastErrorReason()).'</p>';
116119
}
117120
}
118121
}

demo2/consume.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
$samlSettings = new Settings();
1818
$samlResponse = new Response($samlSettings, $_POST['SAMLResponse']);
1919
if ($samlResponse->isValid()) {
20-
echo 'You are: ' . $samlResponse->getNameId() . '<br>';
20+
echo 'You are: ' . htmlentities($samlResponse->getNameId()) . '<br>';
2121
$attributes = $samlResponse->getAttributes();
2222
if (!empty($attributes)) {
2323
echo 'You have the following attributes:<br>';
@@ -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: 2 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

@@ -47,7 +47,7 @@
4747
}
4848
echo '</tbody></table>';
4949
if (!empty($_SESSION['IdPSessionIndex'])) {
50-
echo '<p>The SessionIndex of the IdP is: '.$_SESSION['IdPSessionIndex'].'</p>';
50+
echo '<p>The SessionIndex of the IdP is: '.htmlentities($_SESSION['IdPSessionIndex']).'</p>';
5151
}
5252
} else {
5353
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)