Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Security: Refactoring timing attack prevention into String class and …

…implementing protection for the HMAC Session strategy.
  • Loading branch information...
commit 3b376e611db0e1d4f88c821ac8c76bfdfdd45745 1 parent 7078e8b
@daschl daschl authored
View
14 security/Password.php
@@ -86,8 +86,7 @@ public static function hash($password, $salt = null) {
/**
* Compares a password and its hashed value using PHP's `crypt()`. Rather than a simple string
- * comparison, this method uses a constant-time algorithm to defend against
- * [timing attacks](http://codahale.com/a-lesson-in-timing-attacks/).
+ * comparison, this method uses a constant-time algorithm to defend against timing attacks.
*
* @see lithium\security\Password::hash()
* @see lithium\security\Password::salt()
@@ -96,16 +95,7 @@ public static function hash($password, $salt = null) {
* @return boolean Returns a boolean indicating whether the password is correct.
*/
public static function check($password, $hash) {
- $password = crypt($password, $hash);
- $result = true;
-
- if (($length = strlen($password)) != strlen($hash)) {
- return false;
- }
- for ($i = 0; $i < $length; $i++) {
- $result = $result && ($password[$i] === $hash[$i]);
- }
- return $result;
+ return String::compareConstant(crypt($password, $hash), $hash);
}
/**
View
3  storage/session/strategy/Hmac.php
@@ -11,6 +11,7 @@
use RuntimeException;
use lithium\core\ConfigException;
use lithium\storage\session\strategy\MissingSignatureException;
+use lithium\util\String;
/**
* This strategy allows you to sign your `Session` and / or `Cookie` data with a passphrase
@@ -110,7 +111,7 @@ public function read($data, array $options = array()) {
$currentSignature = $currentData['__signature'];
$signature = static::_signature($currentData);
- if ($signature !== $currentSignature) {
+ if (!String::compareConstant($signature, $currentSignature)) {
$message = "Possible data tampering: HMAC signature does not match data.";
throw new RuntimeException($message);
}
View
6 tests/cases/util/StringTest.php
@@ -464,6 +464,12 @@ public function testHash() {
$this->assertEqual($expected, $result);
}
+ public function testCompareConstant() {
+ $this->assertTrue(String::compareConstant('Foo', 'Foo'));
+ $this->assertFalse(String::compareConstant('Foo', 'foo'));
+ $this->assertFalse(String::compareConstant('1', 1));
+ }
+
/**
* Verifies that `String::insert()` doesn't completely ignore empty values.
*/
View
20 util/String.php
@@ -186,6 +186,26 @@ public static function hash($string, array $options = array()) {
}
/**
+ * Compares two strings in constant time to prevent timing attacks.
+ *
+ * @link http://codahale.com/a-lesson-in-timing-attacks/ More about timing attacks.
+ * @param string $left The left side of the comparison.
+ * @param string $right The right side of the comparison.
+ * @return boolean Returns a boolean indicating whether the two strings are equal.
+ */
+ public static function compareConstant($left, $right) {
+ $result = true;
+
+ if (($length = strlen($left)) != strlen($right)) {
+ return false;
+ }
+ for ($i = 0; $i < $length; $i++) {
+ $result = $result && ($left[$i] === $right[$i]);
+ }
+ return $result;
+ }
+
+ /**
* Replaces variable placeholders inside a string with any given data. Each key
* in the `$data` array corresponds to a variable placeholder name in `$str`.
*
Please sign in to comment.
Something went wrong with that request. Please try again.