Fix length checks in AES_{un,}wrap_key()

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Fix length checks in AES_{un,}wrap_key()

Theo Buehler-4
The spec, https://tools.ietf.org/html/rfc3394, section 2, states that
we need at least two 64 bit blocks for wrapping and, accordingly, three
64 bit blocks for unwrapping. That is: we need at least 16 bytes for
wrapping and 24 bytes for unwrapping.

This also matches the lower bounds that OpenSSL have in their
CRYPTO_128_{un,}wrap() functions.

In fact, if we pass an input with 'inlen < 8' to AES_unwrap_key(),
this results in a segfault since then inlen -= 8 underflows.

Found while playing with the Wycheproof keywrap test vectors.

Index: aes/aes_wrap.c
===================================================================
RCS file: /var/cvs/src/lib/libcrypto/aes/aes_wrap.c,v
retrieving revision 1.10
diff -u -p -r1.10 aes_wrap.c
--- aes/aes_wrap.c 10 Sep 2015 15:56:24 -0000 1.10
+++ aes/aes_wrap.c 17 Oct 2018 23:12:19 -0000
@@ -66,7 +66,8 @@ AES_wrap_key(AES_KEY *key, const unsigne
 {
  unsigned char *A, B[16], *R;
  unsigned int i, j, t;
- if ((inlen & 0x7) || (inlen < 8))
+
+ if ((inlen & 0x7) || (inlen < 16))
  return -1;
  A = B;
  t = 1;
@@ -100,11 +101,10 @@ AES_unwrap_key(AES_KEY *key, const unsig
 {
  unsigned char *A, B[16], *R;
  unsigned int i, j, t;
- inlen -= 8;
- if (inlen & 0x7)
- return -1;
- if (inlen < 8)
+
+ if ((inlen & 0x7) || (inlen < 24))
  return -1;
+ inlen -= 8;
  A = B;
  t = 6 * (inlen >> 3);
  memcpy(A, in, 8);

Reply | Threaded
Open this post in threaded view
|

Re: Fix length checks in AES_{un,}wrap_key()

Brent Cook
This makes sense, ok bcook@

On Wed, Oct 17, 2018 at 6:28 PM Theo Buehler <[hidden email]> wrote:

> The spec, https://tools.ietf.org/html/rfc3394, section 2, states that
> we need at least two 64 bit blocks for wrapping and, accordingly, three
> 64 bit blocks for unwrapping. That is: we need at least 16 bytes for
> wrapping and 24 bytes for unwrapping.
>
> This also matches the lower bounds that OpenSSL have in their
> CRYPTO_128_{un,}wrap() functions.
>
> In fact, if we pass an input with 'inlen < 8' to AES_unwrap_key(),
> this results in a segfault since then inlen -= 8 underflows.
>
> Found while playing with the Wycheproof keywrap test vectors.
>
> Index: aes/aes_wrap.c
> ===================================================================
> RCS file: /var/cvs/src/lib/libcrypto/aes/aes_wrap.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 aes_wrap.c
> --- aes/aes_wrap.c      10 Sep 2015 15:56:24 -0000      1.10
> +++ aes/aes_wrap.c      17 Oct 2018 23:12:19 -0000
> @@ -66,7 +66,8 @@ AES_wrap_key(AES_KEY *key, const unsigne
>  {
>         unsigned char *A, B[16], *R;
>         unsigned int i, j, t;
> -       if ((inlen & 0x7) || (inlen < 8))
> +
> +       if ((inlen & 0x7) || (inlen < 16))
>                 return -1;
>         A = B;
>         t = 1;
> @@ -100,11 +101,10 @@ AES_unwrap_key(AES_KEY *key, const unsig
>  {
>         unsigned char *A, B[16], *R;
>         unsigned int i, j, t;
> -       inlen -= 8;
> -       if (inlen & 0x7)
> -               return -1;
> -       if (inlen < 8)
> +
> +       if ((inlen & 0x7) || (inlen < 24))
>                 return -1;
> +       inlen -= 8;
>         A = B;
>         t = 6 * (inlen >> 3);
>         memcpy(A, in, 8);
>