CMAC + SERPENT/IDEA/RC2 buffer overflow/crash with oversized key

[NIIBE Yutaka]

Hello,

Thank you for your report.

Guido Vranken via Gcrypt-devel <gcrypt-devel at gnupg.org> wrote:
> I think the correct approach is to
> make gcry_mac_setkey() return an error code if the key has an inappropriate
> size.

Something like this?
-- 


diff --git a/cipher/idea.c b/cipher/idea.c
index 0a810818..7f706660 100644
--- a/cipher/idea.c
+++ b/cipher/idea.c
@@ -251,7 +251,9 @@ do_setkey( IDEA_context *c, const byte *key, unsigned int keylen )
     if( selftest_failed )
 	return GPG_ERR_SELFTEST_FAILED;
 
-    assert(keylen == 16);
+    if (keylen != 16)
+      return GPG_ERR_INV_KEYLEN;
+
     c->have_dk = 0;
     expand_key( key, c->ek );
     invert_key( c->ek, c->dk );
diff --git a/cipher/rfc2268.c b/cipher/rfc2268.c
index f018b640..b093f022 100644
--- a/cipher/rfc2268.c
+++ b/cipher/rfc2268.c
@@ -228,6 +228,9 @@ setkey_core (void *context, const unsigned char *key, unsigned int keylen, int w
   if (keylen < 40 / 8)	/* We want at least 40 bits. */
     return GPG_ERR_INV_KEYLEN;
 
+  if (keylen > 128)
+    return GPG_ERR_INV_KEYLEN;
+
   S = (unsigned char *) ctx->S;
 
   for (i = 0; i < keylen; i++)
diff --git a/cipher/serpent.c b/cipher/serpent.c
index 3c5eed2c..d2f7f16e 100644
--- a/cipher/serpent.c
+++ b/cipher/serpent.c
@@ -732,12 +732,15 @@ serpent_subkeys_generate (serpent_key_t key, serpent_subkeys_t subkeys)
 }
 
 /* Initialize CONTEXT with the key KEY of KEY_LENGTH bits.  */
-static void
+static gcry_err_code_t
 serpent_setkey_internal (serpent_context_t *context,
 			 const byte *key, unsigned int key_length)
 {
   serpent_key_t key_prepared;
 
+  if (key_length > 32)
+    return GPG_ERR_INV_KEYLEN;
+
   serpent_key_prepare (key, key_length, key_prepared);
   serpent_subkeys_generate (key_prepared, context->keys);
 
@@ -758,6 +761,7 @@ serpent_setkey_internal (serpent_context_t *context,
 #endif
 
   wipememory (key_prepared, sizeof(key_prepared));
+  return 0;
 }
 
 /* Initialize CTX with the key KEY of KEY_LENGTH bytes.  */
@@ -791,7 +795,7 @@ serpent_setkey (void *ctx,
   if (serpent_test_ret)
     ret = GPG_ERR_SELFTEST_FAILED;
   else
-    serpent_setkey_internal (context, key, key_length);
+    ret = serpent_setkey_internal (context, key, key_length);
 
   return ret;
 }