pq-crystals / dilithium

Other
374 stars 136 forks source link

Compiler warnings with GCC 11 / -Wall #49

Closed thomwiggers closed 2 years ago

thomwiggers commented 3 years ago

Compiling with GCC 11 leads to a ton of compiler warnings, e.g.:

aes256ctr.c:557:64: warning: argument 3 of type ‘const uint8_t *’ {aka ‘const unsigned char *’} declared as a pointer [-Warray-parameter=]
  557 | void aes256ctr_prf(uint8_t *out, size_t outlen, const uint8_t *key, const uint8_t *nonce)
      |                                                 ~~~~~~~~~~~~~~~^~~
In file included from aes256ctr.c:27:
aes256ctr.h:19:34: note: previously declared as an array ‘const uint8_t[32]’ {aka ‘const unsigned char[32]’}
   19 |                    const uint8_t key[32],
      |                    ~~~~~~~~~~~~~~^~~~~~~
thomwiggers commented 3 years ago

Also scary sounding warnings like:

aes256ctr.c:422:19: warning: array subscript 119 is outside array bounds of ‘uint64_t[64]’ {aka ‘long unsigned int[64]’} [-Warray-bounds]
  422 |         q[7] ^= sk[7];
      |                 ~~^~~
marco-palumbi commented 2 years ago

the warning should came from https://github.com/pq-crystals/dilithium/blob/9dddb2a0537734e749ec2c8d4f952cb90cd9e67b/ref/aes256ctr.c#L507

where add_round_key(q, sk_exp + 112); is called with sk_exp + 112 being sk_exp parameter to aes_ctr4x() defined as uint64_t sk_exp[64]

The code should be safe because aes_ctr4x() is always called passing a uint64_t[120] in the sk_exp parameter.

declaring aes_ctr4x in this way should fix:

static void aes_ctr4x(uint8_t out[64], uint32_t ivw[16], uint64_t sk_exp[120])