Non-timing safe comparison in XMLSecurityKey::verifySignature
Opened this issue · 2 comments
In the XMLSecurityKey::verifySignature
method for the self:: HMAC_SHA1
comparison the strcmp function is use to compare the $signature
versus the $expectedSignature
. This function is not a timing safe comparison and should be replaced with something like hash_equals.
public function verifySignature($data, $signature)
{
switch ($this->cryptParams['library']) {
case 'openssl':
return $this->verifyOpenSSL($data, $signature);
case (self::HMAC_SHA1):
$expectedSignature = hash_hmac("sha1", $data, $this->key, true);
return hash_equals($signature, $expectedSignature);
}
}
The hash_equals function was added in PHP 5.6 but this library only requires >=5.4
. Fortunately there's a polyfill that can be used that will use the built-in function if available: https://gist.github.com/christianfutterlieb/3cf85bc3fe16c70c0442
Thanks, I'm taking a look at the changes I am going to be releasing shortly and considering wether just to bump version or not; otherwise will look at the polyfill
Even though having a polyfill is generally a good idea, as it makes your code simpler, you can always use your own function to perform the comparison, and use hash_equals()
if available, or compare manually byte-by-byte. Here's an example.