Added PasswordUtil::secureCompare()

secureCompare() is invulnerable to timing attacks as the comparison of two strings always takes the same time to finish (no early return).

diff --git a/wcfsetup/install/files/lib/data/user/User.class.php b/wcfsetup/install/files/lib/data/user/User.class.php
index da9e9cc430f035841a57c41562d044d114ab05a9..2a2dfc772bfc638722516e30535b47af95d44dda 100644 (file)
--- a/wcfsetup/install/files/lib/data/user/User.class.php
+++ b/wcfsetup/install/files/lib/data/user/User.class.php
@@ -97,7 +97,7 @@ final class User extends DatabaseObject implements IRESTfulResponse, IRouteContr
                        }
                        
                        // password is correct
-                       if ($this->password == PasswordUtil::getDoubleSaltedHash($password, $this->password)) {
+                       if (PasswordUtil::secureCompare($this->password, PasswordUtil::getDoubleSaltedHash($password, $this->password))) {
                                $isValid = true;
                        }
                }
@@ -127,7 +127,7 @@ final class User extends DatabaseObject implements IRESTfulResponse, IRouteContr
         * @return      boolean         password correct
         */
        public function checkCookiePassword($passwordHash) {
-               if (PasswordUtil::isBlowfish($this->password) && ($this->password == PasswordUtil::getSaltedHash($passwordHash, $this->password))) {
+               if (PasswordUtil::isBlowfish($this->password) && PasswordUtil::secureCompare($this->password, PasswordUtil::getSaltedHash($passwordHash, $this->password))) {
                        return true;
                }
                

diff --git a/wcfsetup/install/files/lib/util/PasswordUtil.class.php b/wcfsetup/install/files/lib/util/PasswordUtil.class.php
index 8824878600ae046aeb6ebc2c81ee28be572d15a7..0aabfa8d75c8fabd1d61e4712d48b541858bdbf7 100644 (file)
--- a/wcfsetup/install/files/lib/util/PasswordUtil.class.php
+++ b/wcfsetup/install/files/lib/util/PasswordUtil.class.php
@@ -202,6 +202,28 @@ final class PasswordUtil {
                return str_shuffle($password);
        }
        
+       /**
+        * Compares two password hashes. This function is protected against timing attacks.
+        * 
+        * @see         http://codahale.com/a-lesson-in-timing-attacks/
+        * 
+        * @param       string          $hash1
+        * @param       string          $hash2
+        * @return      boolean
+        */
+       public static function secureCompare($hash1, $hash2) {
+               if (strlen($hash1) !== strlen($hash2)) {
+                       return false;
+               }
+       
+               $result = 0;
+               for ($i = 0, $length = strlen($hash1); $i < $length; $i++) {
+                       $result |= ord($hash1[$i]) ^ ord($hash2[$i]);
+               }
+       
+               return ($result === 0);
+       }
+       
        /**
         * Returns a blowfish salt, e.g. $2a$07$usesomesillystringforsalt$
         * 
@@ -237,7 +259,7 @@ final class PasswordUtil {
         * @return      boolean
         */
        protected static function ipb3($username, $password, $salt, $dbHash) {
-               return ($dbHash == md5(md5($salt) . md5($password)));
+               return self::secureCompare($dbHash, md5(md5($salt) . md5($password)));
        }
        
        /**
@@ -250,7 +272,7 @@ final class PasswordUtil {
         * @return      boolean
         */
        protected static function mybb1($username, $password, $salt, $dbHash) {
-               return ($dbHash == md5(md5($salt) . md5($password)));
+               return self::secureCompare($dbHash, md5(md5($salt) . md5($password)));
        }
        
        /**
@@ -263,7 +285,7 @@ final class PasswordUtil {
         * @return      boolean
         */
        protected static function smf1($username, $password, $salt, $dbHash) {
-               return ($dbHash == sha1(StringUtil::toLowerCase($username) . $password));
+               return self::secureCompare($dbHash, sha1(StringUtil::toLowerCase($username) . $password));
        }
        
        /**
@@ -289,7 +311,7 @@ final class PasswordUtil {
         * @return      boolean
         */
        protected static function vb3($username, $password, $salt, $dbHash) {
-               return ($dbHash == md5(md5($password) . $salt));
+               return self::secureCompare($dbHash, md5(md5($password) . $salt));
        }
        
        /**
@@ -328,10 +350,10 @@ final class PasswordUtil {
         * @return      boolean
         */
        protected static function wbb2($username, $password, $salt, $dbHash) {
-               if ($dbHash == md5($password)) {
+               if (self::secureCompare($dbHash, md5($password))) {
                        return true;
                }
-               else if ($dbHash == sha1($password)) {
+               else if (self::secureCompare($dbHash, sha1($password))) {
                        return true;
                }
                
@@ -348,7 +370,7 @@ final class PasswordUtil {
         * @return      boolean
         */
        protected static function wcf1($username, $password, $salt, $dbHash) {
-               return ($dbHash == sha1($salt . sha1($salt . sha1($password))));
+               return self::secureCompare($dbHash, sha1($salt . sha1($salt . sha1($password))));
        }
        
        /**
@@ -361,7 +383,7 @@ final class PasswordUtil {
         * @return      boolean
         */
        protected static function wcf2($username, $password, $salt, $dbHash) {
-               return ($dbHash == self::getDoubleSaltedHash($password, $salt));
+               return self::secureCompare($dbHash, self::getDoubleSaltedHash($password, $salt));
        }
        
        /**
@@ -374,11 +396,11 @@ final class PasswordUtil {
         * @return      boolean
         */
        protected static function xf1($username, $password, $salt, $dbHash) {
-               if ($dbHash == sha1(sha1($password) . $salt)) {
+               if (self::secureCompare($dbHash, sha1(sha1($password) . $salt))) {
                        return true;
                }
                else if (extension_loaded('hash')) {
-                       return ($dbHash == hash('sha256', hash('sha256', $password) . $salt));
+                       return self::secureCompare($dbHash, hash('sha256', hash('sha256', $password) . $salt));
                }
                
                return false;