diff --git a/framework/base/Security.php b/framework/base/Security.php index 4523e84bed..73ed472d73 100644 --- a/framework/base/Security.php +++ b/framework/base/Security.php @@ -184,12 +184,11 @@ class Security extends Component $calculatedHash = hash_hmac($algorithm, $pureData, $key); - // timing attack resistant approach: - $diff = 0; - for ($i = 0; $i < StringHelper::byteLength($calculatedHash); $i++) { - $diff |= (ord($calculatedHash[$i]) ^ ord($hash[$i])); + if ($this->compareString($hash, $calculatedHash)) { + return $pureData; + } else { + return false; } - return $diff === 0 ? $pureData : false; } else { return false; } @@ -322,14 +321,7 @@ class Security extends Component return false; } - // Use a for-loop to compare two strings to prevent timing attacks. See: - // http://codereview.stackexchange.com/questions/13512 - $check = 0; - for ($i = 0; $i < $n; ++$i) { - $check |= (ord($test[$i]) ^ ord($hash[$i])); - } - - return $check === 0; + return $this->compareString($test, $hash); } /** @@ -365,4 +357,21 @@ class Security extends Component return $salt; } + + /** + * Performs string comparison using timing attack resistant approach. + * @see http://codereview.stackexchange.com/questions/13512 + * @param string $expected string to compare. + * @param string $actual string to compare. + * @return boolean whether strings are equal. + */ + protected function compareString($expected, $actual) + { + // timing attack resistant approach: + $diff = 0; + for ($i = 0; $i < StringHelper::byteLength($actual); $i++) { + $diff |= (ord($actual[$i]) ^ ord($expected[$i])); + } + return $diff === 0; + } } \ No newline at end of file diff --git a/tests/unit/framework/base/SecurityTest.php b/tests/unit/framework/base/SecurityTest.php index 401bb7e9c5..d51db0e81d 100644 --- a/tests/unit/framework/base/SecurityTest.php +++ b/tests/unit/framework/base/SecurityTest.php @@ -24,8 +24,11 @@ class SecurityTest extends TestCase { parent::setUp(); $this->security = new Security(); + $this->security->derivationIterations = 100; // speed up test running } + // Tests : + public function testPasswordHash() { $password = 'secret';