Security issue : use hash_equals() instead of '===' to compare hashes

[iransmaarthbb]

Hi dev,
I would like to point out a security issue in the EncryptService class :

protected function hash_equals($a, $b) {
		if (function_exists('random_bytes')) {
			$key = random_bytes(128);
		} else {
			$key = openssl_random_pseudo_bytes(128);
		}
		return hash_hmac('sha512', $a, $key) === hash_hmac('sha512', $b, $key);
}

A simple strict equals sign === is used for hash comparison, which is vulnerable to timing attack.
The hash_equals() function should be used (http://php.net/manual/en/function.hash-equals.php) for comparing hashes.

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[ckanibal]

This construct is actually called a Double HMAC Comparison with Random Key and should offer reasonable timing attack resistance. More background here: https://paragonie.com/blog/2015/11/preventing-timing-attacks-on-string-comparison-with-double-hmac-strategy

It looks like it is used as a polyfill here, since the native hash_equals is only available from PHP 5.6 onward. However, since the latest Nextcloud depends on PHP7 anyway, it could simply be removed.