Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 1.1.0RC2

Bug #9638 Double calls to decrypt() method with same key do not reinitialize the _P array
Submitted: 2006-12-16 17:42 UTC
From: chad dot wagner at gmail dot com Assigned: mfonda
Status: Closed Package: Crypt_Blowfish (version 1.1.0RC1)
PHP Version: 4.3.11 OS: Linux
Roadmaps: (Not assigned)    
Subscription  


 [2006-12-16 17:42 UTC] chad dot wagner at gmail dot com (Chad Wagner)
Description: ------------ Error is thrown when double calls are made to decrypt: Reason appears to be in Crypt/Blowfish/PHP.php that keyHash is static in setKey. Moving $keyHash to a class variable appears to solve the issue. Test script: --------------- $key = 'blahblah'; $iv = 'halbhalb'; require_once ('Crypt/Blowfish.php'); $c =& Crypt_Blowfish::factory ('cbc', null, null, CRYPT_BLOWFISH_PHP); $c->setKey ($key, $iv); $val = base64_encode ($c->encrypt('cool')); echo $val . "\n"; $c->setKey ($key, $iv); echo $c->decrypt(base64_decode ($val)) . "\n"; $c =& Crypt_Blowfish::factory ('cbc', null, null, CRYPT_BLOWFISH_PHP); $c->setKey ($key, $iv); echo $c->decrypt(base64_decode ($val)) . "\n"; Expected result: ---------------- W2UjbYSOVo8= cool cool Actual result: -------------- W2UjbYSOVo8= cool Object

Comments

 [2006-12-17 18:18 UTC] chad dot wagner at gmail dot com
Here is a patch, basically moved keyHash to a class variable. diff -wurN Crypt_Blowfish-1.1.0RC1/Blowfish/MCrypt.php Crypt/Blowfish/MCrypt.php --- Crypt_Blowfish-1.1.0RC1/Blowfish/MCrypt.php 2006-05-29 13:16:43.000000000 -0400 +++ Crypt/Blowfish/MCrypt.php 2006-12-16 13:39:13.240241469 -0500 @@ -156,8 +156,6 @@ */ function setKey($key, $iv = null) { - static $keyHash = null; - if (!is_string($key)) { return PEAR::raiseError('Key must be a string', 2); } diff -wurN Crypt_Blowfish-1.1.0RC1/Blowfish/PHP.php Crypt/Blowfish/PHP.php --- Crypt_Blowfish-1.1.0RC1/Blowfish/PHP.php 2006-05-29 13:16:43.000000000 -0400 +++ Crypt/Blowfish/PHP.php 2006-12-16 12:43:34.914373766 -0500 @@ -73,6 +73,15 @@ var $_iv_required = false; /** + * Hash value of the previous key + * + * @var string + * @access protected + */ + var $_keyHash = null; + + + /** * Crypt_Blowfish_PHP Constructor * Initializes the Crypt_Blowfish object, and sets * the secret key @@ -203,8 +212,6 @@ */ function setKey($key, $iv = null) { - static $keyHash = null; - if (!is_string($key)) { return PEAR::raiseError('Key must be a string', 2); } @@ -224,7 +231,7 @@ // If same key passed, no need to re-initialize internal arrays. // @todo This needs to be worked out better... - if ($keyHash == md5($key)) { + if ($this->_keyHash == md5($key)) { return true; } @@ -270,7 +277,7 @@ $this->_S[3][$i+1] = $datar; } - $keyHash = md5($key); + $this->_keyHash = md5($key); return true; } }
 [2008-01-25 01:11 UTC] seizethedave (David Grant)
It would be great to have this fix in a real release...
 [2008-08-06 12:50 UTC] koto (Krzysztof Kotowicz)
Similar problem occurs when creating two separate objects using factory method: e.g.: $encryption_key = "<key>"; $blowfish1 = Crypt_Blowfish::factory('ecb', $encryption_key); $blowfish2 = Crypt_Blowfish::factory('ecb', $encryption_key); $blowfish1->encrypt("anything"); // OK $blowfish2->encrypt("anything"); // PEAR Error: the key is not initialized Applying the patch solves the problem.
 [2008-08-07 18:05 UTC] mfonda (Matthew Fonda)
This bug has been fixed in CVS. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better.