Merge branch 'feature/22' into develop

Close #22 Fixes #20
zendframework · Apr 6, 2016 · 9c56dff · 9c56dff
2 parents 36fec44 + 5a01dcf
commit 9c56dff
Show file tree

Hide file tree

Showing 15 changed files with 65 additions and 57 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,7 +6,8 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Added
 
-- Nothing.
+- [#22](https://github.com/zendframework/zend-crypt/pull/22) adds a requirement
+  on `ext/mbstring` in order to install successfully.
 
 ### Deprecated
 
@@ -16,6 +17,12 @@ All notable changes to this project will be documented in this file, in reverse
 
 - Nothing.
 
+### Fixed
+
+- [#22](https://github.com/zendframework/zend-crypt/pull/22) updates all
+  occurrences of `substr()` and `strlen()` to use `mb_substr()` and
+  `mb_strlen()`, respectively. This provides better security with binary values.
+
 ## 2.6.1 - TBD
 
 ### Added

diff --git a/composer.json b/composer.json
@@ -14,6 +14,7 @@
     },
     "require": {
         "php": "^5.5 || ^7.0",
+        "ext-mbstring": "*",
         "zendframework/zend-math": "^2.6",
         "zendframework/zend-stdlib": "^2.7 || ^3.0",
         "container-interop/container-interop": "~1.0"

diff --git a/src/BlockCipher.php b/src/BlockCipher.php
@@ -425,9 +425,9 @@ public function encrypt($data)
             $keySize * 2
         );
         // set the encryption key
-        $this->cipher->setKey(substr($hash, 0, $keySize));
+        $this->cipher->setKey(mb_substr($hash, 0, $keySize, '8bit'));
         // set the key for HMAC
-        $keyHmac = substr($hash, $keySize);
+        $keyHmac = mb_substr($hash, $keySize, null, '8bit');
         // encryption
         $ciphertext = $this->cipher->encrypt($data);
         // HMAC
@@ -461,12 +461,12 @@ public function decrypt($data)
             throw new Exception\InvalidArgumentException('No symmetric cipher specified');
         }
         $hmacSize   = Hmac::getOutputSize($this->hash);
-        $hmac       = substr($data, 0, $hmacSize);
-        $ciphertext = substr($data, $hmacSize) ?: '';
+        $hmac       = mb_substr($data, 0, $hmacSize, '8bit');
+        $ciphertext = mb_substr($data, $hmacSize, null, '8bit') ?: '';
         if (!$this->binaryOutput) {
             $ciphertext = base64_decode($ciphertext);
         }
-        $iv      = substr($ciphertext, 0, $this->cipher->getSaltSize());
+        $iv      = mb_substr($ciphertext, 0, $this->cipher->getSaltSize(), '8bit');
         $keySize = $this->cipher->getKeySize();
         // generate the encryption key and the HMAC key for the authentication
         $hash = Pbkdf2::calc(
@@ -477,9 +477,9 @@ public function decrypt($data)
             $keySize * 2
         );
         // set the decryption key
-        $this->cipher->setKey(substr($hash, 0, $keySize));
+        $this->cipher->setKey(mb_substr($hash, 0, $keySize, '8bit'));
         // set the key for HMAC
-        $keyHmac = substr($hash, $keySize);
+        $keyHmac = mb_substr($hash, $keySize, null, '8bit');
         $hmacNew = Hmac::compute($keyHmac, $this->hash, $this->cipher->getAlgorithm() . $ciphertext);
         if (!Utils::compareStrings($hmacNew, $hmac)) {
             return false;

diff --git a/src/FileCipher.php b/src/FileCipher.php
@@ -241,18 +241,18 @@ public function encrypt($fileIn, $fileOut)
         $tot     = filesize($fileIn);
         $padding = $this->cipher->getPadding();
 
-        $this->cipher->setKey(substr($keys, 0, $this->cipher->getKeySize()));
+        $this->cipher->setKey(mb_substr($keys, 0, $this->cipher->getKeySize(), '8bit'));
         $this->cipher->setPadding(new Symmetric\Padding\NoPadding);
         $this->cipher->setSalt($iv);
         $this->cipher->setMode('cbc');
 
         $hashAlgo  = $this->getHashAlgorithm();
         $saltSize  = $this->cipher->getSaltSize();
         $algorithm = $this->cipher->getAlgorithm();
-        $keyHmac   = substr($keys, $this->cipher->getKeySize());
+        $keyHmac   = mb_substr($keys, $this->cipher->getKeySize(), null, '8bit');
 
         while ($data = fread($read, self::BUFFER_SIZE)) {
-            $size += strlen($data);
+            $size += mb_strlen($data, '8bit');
             // Padding if last block
             if ($size == $tot) {
                 $this->cipher->setPadding($padding);
@@ -262,20 +262,20 @@ public function encrypt($fileIn, $fileOut)
                 // Write a placeholder for the HMAC and write the IV
                 fwrite($write, str_repeat(0, Hmac::getOutputSize($hashAlgo)));
             } else {
-                $result = substr($result, $saltSize);
+                $result = mb_substr($result, $saltSize, null, '8bit');
             }
             $hmac = Hmac::compute($keyHmac,
                                   $hashAlgo,
                                   $algorithm . $hmac . $result);
-            $this->cipher->setSalt(substr($result, -1 * $saltSize));
-            if (fwrite($write, $result) !== strlen($result)) {
+            $this->cipher->setSalt(mb_substr($result, -1 * $saltSize, null, '8bit'));
+            if (fwrite($write, $result) !== mb_strlen($result, '8bit')) {
                 return false;
             }
         }
         $result = true;
         // write the HMAC at the beginning of the file
         fseek($write, 0);
-        if (fwrite($write, $hmac) !== strlen($hmac)) {
+        if (fwrite($write, $hmac) !== mb_strlen($hmac, '8bit')) {
             $result = false;
         }
         fclose($write);
@@ -306,25 +306,25 @@ public function decrypt($fileIn, $fileOut)
         $iv       = fread($read, $this->cipher->getSaltSize());
         $tot      = filesize($fileIn);
         $hmac     = $iv;
-        $size     = strlen($iv) + strlen($hmacRead);
+        $size     = mb_strlen($iv, '8bit') + mb_strlen($hmacRead, '8bit');
         $keys     = Pbkdf2::calc($this->getPbkdf2HashAlgorithm(),
                                  $this->getKey(),
                                  $iv,
                                  $this->getKeyIteration(),
                                  $this->cipher->getKeySize() * 2);
         $padding  = $this->cipher->getPadding();
         $this->cipher->setPadding(new Symmetric\Padding\NoPadding);
-        $this->cipher->setKey(substr($keys, 0, $this->cipher->getKeySize()));
+        $this->cipher->setKey(mb_substr($keys, 0, $this->cipher->getKeySize(), '8bit'));
         $this->cipher->setMode('cbc');
 
         $blockSize = $this->cipher->getBlockSize();
         $hashAlgo  = $this->getHashAlgorithm();
         $algorithm = $this->cipher->getAlgorithm();
         $saltSize  = $this->cipher->getSaltSize();
-        $keyHmac   = substr($keys, $this->cipher->getKeySize());
+        $keyHmac   = mb_substr($keys, $this->cipher->getKeySize(), null, '8bit');
 
         while ($data = fread($read, self::BUFFER_SIZE)) {
-            $size += strlen($data);
+            $size += mb_strlen($data, '8bit');
             // Unpadding if last block
             if ($size + $blockSize >= $tot) {
                 $this->cipher->setPadding($padding);
@@ -334,8 +334,8 @@ public function decrypt($fileIn, $fileOut)
             $hmac   = Hmac::compute($keyHmac,
                                     $hashAlgo,
                                     $algorithm . $hmac . $data);
-            $iv     = substr($data, -1 * $saltSize);
-            if (fwrite($write, $result) !== strlen($result)) {
+            $iv     = mb_substr($data, -1 * $saltSize, null, '8bit');
+            if (fwrite($write, $result) !== mb_strlen($result, '8bit')) {
                 return false;
             }
         }

diff --git a/src/Hash.php b/src/Hash.php
@@ -48,7 +48,7 @@ public static function compute($hash, $data, $output = self::OUTPUT_STRING)
      */
     public static function getOutputSize($hash, $output = self::OUTPUT_STRING)
     {
-        return strlen(static::compute($hash, 'data', $output));
+        return mb_strlen(static::compute($hash, 'data', $output), '8bit');
     }
 
     /**

diff --git a/src/Hmac.php b/src/Hmac.php
@@ -60,7 +60,7 @@ public static function compute($key, $hash, $data, $output = self::OUTPUT_STRING
      */
     public static function getOutputSize($hash, $output = self::OUTPUT_STRING)
     {
-        return strlen(static::compute('key', $hash, 'data', $output));
+        return mb_strlen(static::compute('key', $hash, 'data', $output), '8bit');
     }
 
     /**

diff --git a/src/Key/Derivation/Pbkdf2.php b/src/Key/Derivation/Pbkdf2.php
@@ -44,6 +44,6 @@ public static function calc($hash, $password, $salt, $iterations, $length)
             }
             $result .= $mix;
         }
-        return substr($result, 0, $length);
+        return mb_substr($result, 0, $length, '8bit');
     }
 }
diff --git a/src/Key/Derivation/SaltedS2k.php b/src/Key/Derivation/SaltedS2k.php
@@ -64,7 +64,7 @@ public static function calc($hash, $password, $salt, $bytes)
         if (!in_array($hash, array_keys(static::$supportedMhashAlgos))) {
             throw new Exception\InvalidArgumentException("The hash algorithm $hash is not supported by " . __CLASS__);
         }
-        if (strlen($salt)<8) {
+        if (mb_strlen($salt, '8bit') < 8) {
             throw new Exception\InvalidArgumentException('The salt size must be at least of 8 bytes');
         }
         return mhash_keygen_s2k(static::$supportedMhashAlgos[$hash], $password, $salt, $bytes);

diff --git a/src/Key/Derivation/Scrypt.php b/src/Key/Derivation/Scrypt.php
@@ -51,7 +51,7 @@ public static function calc($password, $salt, $n, $r, $p, $length)
 
         $s = '';
         for ($i = 0; $i < $p; $i++) {
-            $s .= self::scryptROMix(substr($b, $i * 128 * $r, 128 * $r), $n, $r);
+            $s .= self::scryptROMix(mb_substr($b, $i * 128 * $r, 128 * $r, '8bit'), $n, $r);
         }
 
         return Pbkdf2::calc('sha256', $password, $s, 1, $length);
@@ -92,16 +92,16 @@ protected static function scryptROMix($b, $n, $r)
      */
     protected static function scryptBlockMix($b, $r)
     {
-        $x    = substr($b, -64);
+        $x    = mb_substr($b, -64, null, '8bit');
         $even = '';
         $odd  = '';
         $len  = 2 * $r;
 
         for ($i = 0; $i < $len; $i++) {
             if (PHP_INT_SIZE === 4) {
-                $x = self::salsa208Core32($x ^ substr($b, 64 * $i, 64));
+                $x = self::salsa208Core32($x ^ mb_substr($b, 64 * $i, 64, '8bit'));
             } else {
-                $x = self::salsa208Core64($x ^ substr($b, 64 * $i, 64));
+                $x = self::salsa208Core64($x ^ mb_substr($b, 64 * $i, 64, '8bit'));
             }
             if ($i % 2 == 0) {
                 $even .= $x;
@@ -124,7 +124,7 @@ protected static function salsa208Core32($b)
     {
         $b32 = [];
         for ($i = 0; $i < 16; $i++) {
-            list(, $b32[$i]) = unpack("V", substr($b, $i * 4, 4));
+            list(, $b32[$i]) = unpack("V", mb_substr($b, $i * 4, 4, '8bit'));
         }
 
         $x = $b32;
@@ -217,7 +217,7 @@ protected static function salsa208Core64($b)
     {
         $b32 = [];
         for ($i = 0; $i < 16; $i++) {
-            list(, $b32[$i]) = unpack("V", substr($b, $i * 4, 4));
+            list(, $b32[$i]) = unpack("V", mb_substr($b, $i * 4, 4, '8bit'));
         }
 
         $x = $b32;
@@ -315,7 +315,7 @@ protected static function integerify($b)
         if (PHP_INT_SIZE === 8) {
             $v = 'V';
         }
-        list(, $n) = unpack($v, substr($b, -64));
+        list(, $n) = unpack($v, mb_substr($b, -64, null, '8bit'));
         return $n;
     }
 }
diff --git a/src/Password/Apache.php b/src/Password/Apache.php
@@ -125,12 +125,12 @@ public function create($password)
      */
     public function verify($password, $hash)
     {
-        if (substr($hash, 0, 5) === '{SHA}') {
+        if (mb_substr($hash, 0, 5, '8bit') === '{SHA}') {
             $hash2 = '{SHA}' . base64_encode(sha1($password, true));
             return Utils::compareStrings($hash, $hash2);
         }
 
-        if (substr($hash, 0, 6) === '$apr1$') {
+        if (mb_substr($hash, 0, 6, '8bit') === '$apr1$') {
             $token = explode('$', $hash);
             if (empty($token[2])) {
                 throw new Exception\InvalidArgumentException(
@@ -143,7 +143,7 @@ public function verify($password, $hash)
 
         $bcryptPattern = '/\$2[ay]?\$[0-9]{2}\$[' . addcslashes(static::BASE64, '+/') . '\.]{53}/';
 
-        if (strlen($hash) > 13 && ! preg_match($bcryptPattern, $hash)) { // digest
+        if (mb_strlen($hash, '8bit') > 13 && ! preg_match($bcryptPattern, $hash)) { // digest
             if (empty($this->userName) || empty($this->authName)) {
                 throw new Exception\RuntimeException(
                     'You must specify UserName and AuthName (realm) to verify the digest'
@@ -242,7 +242,7 @@ public function getUserName()
      */
     protected function toAlphabet64($value)
     {
-        return strtr(strrev(substr(base64_encode($value), 2)), self::BASE64, self::ALPHA64);
+        return strtr(strrev(mb_substr(base64_encode($value), 2, null, '8bit')), self::BASE64, self::ALPHA64);
     }
 
     /**
@@ -257,7 +257,7 @@ protected function apr1Md5($password, $salt = null)
         if (null === $salt) {
             $salt = Rand::getString(8, self::ALPHA64);
         } else {
-            if (strlen($salt) !== 8) {
+            if (mb_strlen($salt, '8bit') !== 8) {
                 throw new Exception\InvalidArgumentException(
                     'The salt value for APR1 algorithm must be 8 characters long'
                 );
@@ -270,11 +270,11 @@ protected function apr1Md5($password, $salt = null)
                 }
             }
         }
-        $len  = strlen($password);
+        $len  = mb_strlen($password, '8bit');
         $text = $password . '$apr1$' . $salt;
         $bin  = pack("H32", md5($password . $salt . $password));
         for ($i = $len; $i > 0; $i -= 16) {
-            $text .= substr($bin, 0, min(16, $i));
+            $text .= mb_substr($bin, 0, min(16, $i), '8bit');
         }
         for ($i = $len; $i > 0; $i >>= 1) {
             $text .= ($i & 1) ? chr(0) : $password[0];

diff --git a/src/Password/Bcrypt.php b/src/Password/Bcrypt.php
@@ -78,14 +78,14 @@ public function create($password)
         } else {
             $salt = $this->salt;
         }
-        $salt64 = substr(str_replace('+', '.', base64_encode($salt)), 0, 22);
+        $salt64 = mb_substr(str_replace('+', '.', base64_encode($salt)), 0, 22, '8bit');
         /**
          * Check for security flaw in the bcrypt implementation used by crypt()
          * @see http://php.net/security/crypt_blowfish.php
          */
         $prefix = '$2y$';
         $hash = crypt($password, $prefix . $this->cost . '$' . $salt64);
-        if (strlen($hash) < 13) {
+        if (mb_strlen($hash, '8bit') < 13) {
             throw new Exception\RuntimeException('Error during the bcrypt generation');
         }
         return $hash;
@@ -145,7 +145,7 @@ public function getCost()
      */
     public function setSalt($salt)
     {
-        if (strlen($salt) < self::MIN_SALT_SIZE) {
+        if (mb_strlen($salt, '8bit') < self::MIN_SALT_SIZE) {
             throw new Exception\InvalidArgumentException(
                 'The length of the salt must be at least ' . self::MIN_SALT_SIZE . ' bytes'
             );

diff --git a/src/PublicKey/DiffieHellman.php b/src/PublicKey/DiffieHellman.php
@@ -433,6 +433,6 @@ protected function convert($number, $inputFormat = self::FORMAT_NUMBER, $outputF
      */
     protected function generatePrivateKey()
     {
-        return Math\Rand::getBytes(strlen($this->getPrime()), true);
+        return Math\Rand::getBytes(mb_strlen($this->getPrime(), '8bit'), true);
     }
 }
diff --git a/src/Symmetric/Mcrypt.php b/src/Symmetric/Mcrypt.php
@@ -237,7 +237,7 @@ public function getKeySize()
      */
     public function setKey($key)
     {
-        $keyLen = strlen($key);
+        $keyLen = mb_strlen($key, '8bit');
 
         if (!$keyLen) {
             throw new Exception\InvalidArgumentException('The key cannot be empty');
@@ -271,7 +271,7 @@ public function getKey()
         if (empty($this->key)) {
             return;
         }
-        return substr($this->key, 0, $this->getKeySize());
+        return mb_substr($this->key, 0, $this->getKeySize(), '8bit');
     }
 
     /**
@@ -381,8 +381,8 @@ public function decrypt($data)
         if (null === $this->getPadding()) {
             throw new Exception\InvalidArgumentException('You have to specify a padding method');
         }
-        $iv         = substr($data, 0, $this->getSaltSize());
-        $ciphertext = substr($data, $this->getSaltSize());
+        $iv         = mb_substr($data, 0, $this->getSaltSize(), '8bit');
+        $ciphertext = mb_substr($data, $this->getSaltSize(), null, '8bit');
         $result     = mcrypt_decrypt(
             $this->supportedAlgos[$this->algo],
             $this->getKey(),
@@ -426,7 +426,7 @@ public function setSalt($salt)
         if (empty($salt)) {
             throw new Exception\InvalidArgumentException('The salt (IV) cannot be empty');
         }
-        if (strlen($salt) < $this->getSaltSize()) {
+        if (mb_strlen($salt, '8bit') < $this->getSaltSize()) {
             throw new Exception\InvalidArgumentException(
                 'The size of the salt (IV) must be at least ' . $this->getSaltSize() . ' bytes'
             );
@@ -446,13 +446,13 @@ public function getSalt()
         if (empty($this->iv)) {
             return;
         }
-        if (strlen($this->iv) < $this->getSaltSize()) {
+        if (mb_strlen($this->iv, '8bit') < $this->getSaltSize()) {
             throw new Exception\RuntimeException(
                 'The size of the salt (IV) must be at least ' . $this->getSaltSize() . ' bytes'
             );
         }
 
-        return substr($this->iv, 0, $this->getSaltSize());
+        return mb_substr($this->iv, 0, $this->getSaltSize(), '8bit');
     }
 
     /**
-Original file line number
+Diff line change
@@ Expand Up @@
          */
         public static function getOutputSize($hash, $output = self::OUTPUT_STRING)
         {
-            return strlen(static::compute($hash, 'data', $output));
+            return mb_strlen(static::compute($hash, 'data', $output), '8bit');
         }
         /**
@@ Expand Down @@