Skip to content

Commit

Permalink
Security update for signature validation on LogoutRequest/LogoutRespo…
Browse files Browse the repository at this point in the history
…nse.

In order to verify Signatures on Logoutrequests and LogoutResponses we use
the verifySignature of the class XMLSecurityKey from the xmlseclibs library.
That method end up calling openssl_verify() depending on the signature algorithm used.

The openssl_verify() function returns 1 when the signature was successfully verified,
0 if it failed to verify with the given key, and -1 in case an error occurs.
PHP allows translating numerical values to boolean implicitly, with the following correspondences:
- 0 equals false.
- Non-zero equals true.

This means that an implicit conversion to boolean of the values returned by openssl_verify()
will convert an error state, signaled by the value -1, to a successful verification of the
signature (represented by the boolean true).

The LogoutRequest/LogoutResponse signature validator was performing an implicit conversion to boolean
of the values returned by the verify() method, which subsequently will return the same output
as openssl_verify() under most circumstances.
This means an error during signature verification is treated as a successful verification by the method.

Since the signature validation of SAMLResponses were not affected, the impact of this security
vulnerability is lower, but an update of the php-saml toolkit is recommended.
  • Loading branch information
pitbulk committed Feb 28, 2017
1 parent 595eeba commit 949359f
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/Saml2/LogoutRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ public function isValid($retrieveParametersFromServer=false)
}
}

if (!$objKey->verifySignature($signedQuery, base64_decode($_GET['Signature']))) {
if ($objKey->verifySignature($signedQuery, base64_decode($_GET['Signature'])) !== 1) {
throw new OneLogin_Saml2_ValidationError(
"Signature validation failed. Logout Request rejected",
OneLogin_Saml2_ValidationError::INVALID_SIGNATURE
Expand Down
2 changes: 1 addition & 1 deletion lib/Saml2/LogoutResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public function isValid($requestId = null, $retrieveParametersFromServer=false)
}
}

if (!$objKey->verifySignature($signedQuery, base64_decode($_GET['Signature']))) {
if ($objKey->verifySignature($signedQuery, base64_decode($_GET['Signature'])) !== 1) {
throw new OneLogin_Saml2_ValidationError(
"Signature validation failed. Logout Response rejected",
OneLogin_Saml2_ValidationError::INVALID_SIGNATURE
Expand Down

0 comments on commit 949359f

Please sign in to comment.