[bug report] crypto: dh - Add DH software implementation

[bug report] crypto: dh - Add DH software implementation

[Dan Carpenter]

Hello Salvatore Benedetto,

The patch 802c7f1c84e4: "crypto: dh - Add DH software implementation"
from Jun 22, 2016, leads to the following static checker warning:

	crypto/dh_helper.c:99 crypto_dh_decode_key()
	warn: potential overflow

crypto/dh_helper.c
    68  int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
    69  {
    70          const u8 *ptr = buf;
    71          struct kpp_secret secret;
    72  
    73          if (unlikely(!buf || len < DH_KPP_SECRET_MIN_SIZE))
    74                  return -EINVAL;
    75  
    76          ptr = dh_unpack_data(&secret, ptr, sizeof(secret));
    77          if (secret.type != CRYPTO_KPP_SECRET_TYPE_DH)
    78                  return -EINVAL;
    79  
    80          ptr = dh_unpack_data(&params->key_size, ptr, sizeof(params->key_size));
    81          ptr = dh_unpack_data(&params->p_size, ptr, sizeof(params->p_size));
    82          ptr = dh_unpack_data(&params->q_size, ptr, sizeof(params->q_size));
    83          ptr = dh_unpack_data(&params->g_size, ptr, sizeof(params->g_size));
    84          if (secret.len != crypto_dh_key_len(params))

The largest parameter has to be "params->p_size" but it's a u32 from the
user.  So crypto_dh_key_len() can have an integer overflow and wrap back
to "secret.len".

    85                  return -EINVAL;
    86  
    87          /*
    88           * Don't permit the buffer for 'key' or 'g' to be larger than 'p', since
    89           * some drivers assume otherwise.
    90           */
    91          if (params->key_size > params->p_size ||
    92              params->g_size > params->p_size || params->q_size > params->p_size)

This ensures that "params->p_size" is the largest.

    93                  return -EINVAL;
    94  
    95          /* Don't allocate memory. Set pointers to data within
    96           * the given buffer
    97           */
    98          params->key = (void *)ptr;
    99          params->p = (void *)(ptr + params->key_size);
   100          params->q = (void *)(ptr + params->key_size + params->p_size);

This could wrap.

   101          params->g = (void *)(ptr + params->key_size + params->p_size +
   102                               params->q_size);
   103  
   104          /*
   105           * Don't permit 'p' to be 0.  It's not a prime number, and it's subject
   106           * to corner cases such as 'mod 0' being undefined or
   107           * crypto_kpp_maxsize() returning 0.
   108           */
   109          if (memchr_inv(params->p, 0, params->p_size) == NULL)

It would probably/hopefully lead to an Oops in memchr_inv().

   110                  return -EINVAL;
   111  
   112          /* It is permissible to not provide Q. */
   113          if (params->q_size == 0)
   114                  params->q = NULL;
   115  
   116          return 0;
   117  }

regards,
dan carpenter