potential underfow in crypto/lrw.c setkey() setkey

potential underfow in crypto/lrw.c setkey() setkey

[Dan Carpenter]

crypto/lrw.c
    72  static int setkey(struct crypto_skcipher *parent, const u8 *key,
    73                    unsigned int keylen)
    74  {
    75          struct priv *ctx = crypto_skcipher_ctx(parent);
    76          struct crypto_skcipher *child = ctx->child;
    77          int err, bsize = LRW_BLOCK_SIZE;
    78          const u8 *tweak = key + keylen - bsize;
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Smatch thinks that keylen is user controlled from zero to some upper
bound.  How do we know it's >= LRW_BLOCK_SIZE (16)?

I find the crypto code sort of hard to follow...  There are a bunch of
setkey pointers and they're sometimes called recursively.  Is there
some trick or hints?

    79          be128 tmp = { 0 };
    80          int i;
    81  
    82          crypto_skcipher_clear_flags(child, CRYPTO_TFM_REQ_MASK);
    83          crypto_skcipher_set_flags(child, crypto_skcipher_get_flags(parent) &
    84                                           CRYPTO_TFM_REQ_MASK);
    85          err = crypto_skcipher_setkey(child, key, keylen - bsize);
    86          crypto_skcipher_set_flags(parent, crypto_skcipher_get_flags(child) &
    87                                            CRYPTO_TFM_RES_MASK);
    88          if (err)
    89                  return err;
    90  
    91          if (ctx->table)
    92                  gf128mul_free_64k(ctx->table);
    93  
    94          /* initialize multiplication table for Key2 */
    95          ctx->table = gf128mul_init_64k_bbe((be128 *)tweak);
    96          if (!ctx->table)
    97                  return -ENOMEM;
    98  
    99          /* initialize optimization table */
   100          for (i = 0; i < 128; i++) {
   101                  setbit128_bbe(&tmp, i);
   102                  ctx->mulinc[i] = tmp;
   103                  gf128mul_64k_bbe(&ctx->mulinc[i], ctx->table);
   104          }
   105  
   106          return 0;
   107  }

regards,
dan carpenter

Re: potential underfow in crypto/lrw.c setkey() setkey

[Eric Biggers]

On Thu, May 09, 2019 at 02:06:22PM +0300, Dan Carpenter wrote:
> crypto/lrw.c
>     72  static int setkey(struct crypto_skcipher *parent, const u8 *key,
>     73                    unsigned int keylen)
>     74  {
>     75          struct priv *ctx = crypto_skcipher_ctx(parent);
>     76          struct crypto_skcipher *child = ctx->child;
>     77          int err, bsize = LRW_BLOCK_SIZE;
>     78          const u8 *tweak = key + keylen - bsize;
>                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Smatch thinks that keylen is user controlled from zero to some upper
> bound.  How do we know it's >= LRW_BLOCK_SIZE (16)?

See create() in crypto/lrw.c, which is the function that creates an LRW
instance.  It sets .min_keysize and .max_keysize:

        inst->alg.min_keysize = crypto_skcipher_alg_min_keysize(alg) +
                                LRW_BLOCK_SIZE;
        inst->alg.max_keysize = crypto_skcipher_alg_max_keysize(alg) +
                                LRW_BLOCK_SIZE;

Then when sometime calls crypto_skcipher_setkey(), it calls skcipher_setkey() in
crypto/skcipher.c which verifies the key length is in within the bounds the
algorithm declares:

        if (keylen < cipher->min_keysize || keylen > cipher->max_keysize) {
                crypto_skcipher_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
                return -EINVAL;
        }

> 
> I find the crypto code sort of hard to follow...  There are a bunch of
> setkey pointers and they're sometimes called recursively.  Is there
> some trick or hints?
> 
>     79          be128 tmp = { 0 };
>     80          int i;
>     81  
>     82          crypto_skcipher_clear_flags(child, CRYPTO_TFM_REQ_MASK);
>     83          crypto_skcipher_set_flags(child, crypto_skcipher_get_flags(parent) &
>     84                                           CRYPTO_TFM_REQ_MASK);
>     85          err = crypto_skcipher_setkey(child, key, keylen - bsize);
>     86          crypto_skcipher_set_flags(parent, crypto_skcipher_get_flags(child) &
>     87                                            CRYPTO_TFM_RES_MASK);
>     88          if (err)
>     89                  return err;

LRW is a template for a block cipher mode of operation which is implemented on
top of ECB.  So, LRW's setkey() method calls setkey() on the underlying ECB
transform.  Similarly, ECB then may call setkey() of the underlying block cipher
such as AES, or alternatively it may be implemented directly.

- Eric