From 949359f5cad5e1d085c4e5447d9aa8f49a6e82a1 Mon Sep 17 00:00:00 2001 From: Sixto Martin Date: Tue, 28 Feb 2017 17:16:58 +0100 Subject: [PATCH] Security update for signature validation on LogoutRequest/LogoutResponse. 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. --- lib/Saml2/LogoutRequest.php | 2 +- lib/Saml2/LogoutResponse.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Saml2/LogoutRequest.php b/lib/Saml2/LogoutRequest.php index a99d22b6..457cf501 100644 --- a/lib/Saml2/LogoutRequest.php +++ b/lib/Saml2/LogoutRequest.php @@ -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 diff --git a/lib/Saml2/LogoutResponse.php b/lib/Saml2/LogoutResponse.php index 2246e03e..0637d990 100644 --- a/lib/Saml2/LogoutResponse.php +++ b/lib/Saml2/LogoutResponse.php @@ -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