Same happens in mcrypt_ecb, mcrypt_cbc, mcrypt_cfb and mcrypt_ofb too.


Script showing the bug:

<?php
$key1  = pack("H*", "FF00FF00000000000000000000000000000000000000000000000000000
00000");
$key2  = pack("H*", "FF000000000000000000000000000000000000000000000000000000000
00000");
$iv    = pack("H*", "00000000000000000000000000000000");
$plain = pack("H*", "0000000000000000");

$handle = mcrypt_module_open(MCRYPT_TWOFISH, "", MCRYPT_MODE_CFB, "");
mcrypt_generic_init($handle, $key1, $iv);
$crypted1 = mcrypt_generic($handle, $plain);
mcrypt_generic_end($handle);

$handle = mcrypt_module_open(MCRYPT_TWOFISH, "", MCRYPT_MODE_CFB, "");
mcrypt_generic_init($handle, $key2, $iv);
$crypted2 = mcrypt_generic($handle, $plain);
mcrypt_generic_end($handle);

print bin2hex($plain)."\n\n";
print bin2hex($crypted1)."\n\n";
print bin2hex($crypted2)."\n\n";
?>

The two ciphertexts should NOT be the same as the key is different.


Proposed patch (also fixes a possible memory access problem, but only for the mcrypt_generic_init function, the I didn't fully understand php_mcrypt_do_crypt yet, when I do I will update the patch, see also Bug #10518):

--- php-4.0.4pl1/ext/mcrypt/mcrypt.c    Wed Nov 22 22:40:15 2000
+++ php-4.0.4pl1-sk/ext/mcrypt/mcrypt.c Sat Apr 28 18:53:07 2001
@@ -463,14 +463,22 @@
                        Z_STRLEN_PP(key), key_size);
                php_error (E_NOTICE, dummy);
        }
-       strncpy (key_s, Z_STRVAL_PP(key), key_size);
+       if (Z_STRLEN_PP(key) > key_size) {
+               memcpy (key_s, Z_STRVAL_PP(key), key_size);
+       } else {
+               memcpy (key_s, Z_STRVAL_PP(key), Z_STRLEN_PP(key));
+       }
 
        if (Z_STRLEN_PP(iv) != iv_size) {
                sprintf (dummy, "iv size incorrect; supplied length: %d, needed: %d", 
                        Z_STRLEN_PP(iv), iv_size);
                php_error (E_WARNING, dummy);
        }
-       strncpy (iv_s, Z_STRVAL_PP(iv), iv_size);
+       if (Z_STRLEN_PP(iv) > iv_size) {
+               memcpy (iv_s, Z_STRVAL_PP(iv), iv_size);
+       } else {
+               memcpy (iv_s, Z_STRVAL_PP(iv), Z_STRLEN_PP(iv));
+       }
 
        RETVAL_LONG (mcrypt_generic_init (td, key_s, key_size, iv_s));
        efree (iv_s);