In the program below, each of three calls to cmac() causes a different crash (use AddressSanitizer to be sure). I think the correct approach is to make gcry_mac_setkey() return an error code if the key has an inappropriate size. #include <gcrypt.h> #define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; } static void cmac(const int mac, const int keysize) { unsigned char key[keysize]; memset(key, 0, keysize); gcry_mac_hd_t h; CF_CHECK_EQ(gcry_mac_open(&h, mac, 0, NULL), GPG_ERR_NO_ERROR); CF_CHECK_EQ(gcry_mac_setkey(h, key, keysize), GPG_ERR_NO_ERROR); end: /* noret */ gcry_mac_close(h); } int main(void) { cmac(GCRY_MAC_CMAC_SERPENT, 64); cmac(GCRY_MAC_CMAC_IDEA, 32); cmac(GCRY_MAC_CMAC_RFC2268, 256); return 0; } _______________________________________________ Gcrypt-devel mailing list [hidden email] http://lists.gnupg.org/mailman/listinfo/gcrypt-devel |
Hello,
Thank you for your report. Guido Vranken via Gcrypt-devel <[hidden email]> 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; } _______________________________________________ Gcrypt-devel mailing list [hidden email] http://lists.gnupg.org/mailman/listinfo/gcrypt-devel |
NIIBE Yutaka <[hidden email]> wrote:
> Something like this? After I confirmed it works correctly (returns an error), I applied and pushed to libgcrypt master. -- _______________________________________________ Gcrypt-devel mailing list [hidden email] http://lists.gnupg.org/mailman/listinfo/gcrypt-devel |
In reply to this post by GnuPG - Libgcrypt - Dev mailing list
On 2021-03-31 Guido Vranken via Gcrypt-devel <[hidden email]> wrote:
> In the program below, each of three calls to cmac() causes a different > crash (use AddressSanitizer to be sure). I think the correct approach is to > make gcry_mac_setkey() return an error code if the key has an inappropriate > size. [...] Is this exploitable? cu Andreas -- `What a good friend you are to him, Dr. Maturin. His other friends are so grateful to you.' `I sew his ears on from time to time, sure' _______________________________________________ Gcrypt-devel mailing list [hidden email] http://lists.gnupg.org/mailman/listinfo/gcrypt-devel |
In the case of IDEA, an oversized key only appears to cause a crash (assertion failure, so no memory corruption). In the case of SERPENT, it appears to overwrite a stack buffer. That makes it unlikely to be exploitable when stack protector is enabled (which is the case for the binaries of most Linux distro's I think). In the case of RC2, it appears to overwrite a heap buffer. That makes it more likely to be exploitable but you probably still have to deal with ASLR. In addition to that the host application needs to allow setting oversized keys but applications usually only allow hardcoded, legal key lengths. With that said, exploitation might be possible in specific circumstances. On Fri, Apr 2, 2021 at 6:00 PM Andreas Metzler <[hidden email]> wrote: On 2021-03-31 Guido Vranken via Gcrypt-devel <[hidden email]> wrote: _______________________________________________ Gcrypt-devel mailing list [hidden email] http://lists.gnupg.org/mailman/listinfo/gcrypt-devel |
On Fri, 2 Apr 2021 22:51, Guido Vranken said:
> With that said, exploitation might be possible in specific circumstances. ... and it would be much easier to attack the application than Libgcrypt. An application which does not take care from where it gets the key has for sure a lot of other problems. Shalom-Salam, Werner -- Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz. _______________________________________________ Gcrypt-devel mailing list [hidden email] http://lists.gnupg.org/mailman/listinfo/gcrypt-devel |
Free forum by Nabble | Edit this page |