Mailing List Archive

Hello,

Thank you for your report.

Guido Vranken via Gcrypt-devel <gcrypt-devel@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;
}

_______________________________________________
Gcrypt-devel mailing list
Gcrypt-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel

NIIBE Yutaka <gniibe@fsij.org> wrote:
> Something like this?

After I confirmed it works correctly (returns an error), I applied and
pushed to libgcrypt master.
--

_______________________________________________
Gcrypt-devel mailing list
Gcrypt-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gcrypt-devel

On 2021-03-31 Guido Vranken via Gcrypt-devel <gcrypt-devel@gnupg.org> 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
Gcrypt-devel@gnupg.org
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 <ametzler@bebt.de> wrote:

> On 2021-03-31 Guido Vranken via Gcrypt-devel <gcrypt-devel@gnupg.org>
> 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
> Gcrypt-devel@gnupg.org
> 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.