eclipse / tinydtls

Eclipse tinydtls
https://projects.eclipse.org/projects/iot.tinydtls
Other
105 stars 57 forks source link

Build fails when using -O3 instead of -O2 gcc optimizer #207

Closed mrdeep1 closed 1 year ago

mrdeep1 commented 1 year ago

In this case, this is a false positive (-Wmaybe-uninitialized is too enthusiastic), but it is correct in that dtls_ecdh_pre_master_secret() can get called with a key_sizethat is not exactly 8. key_size < 8 does not properly initialize things, > 8 stack gets trashed. Using Ubuntu 20, gcc version 11.4.0.

$ gcc -Wall -pedantic -std=c99 -DSHA2_USE_INTTYPES_H -g -O3 -fPIC -pedantic -Wall -Wextra -Wformat-security -Winline -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wshadow -Wstrict-prototypes -Wswitch-default -Wswitch-enum -Wunused   -DDTLSv12 -DWITH_SHA256 -DDTLS_CHECK_CONTENTTYPE -I.  -c -o crypto.o crypto.c
In file included from crypto.c:34:
In function ‘ecc_ecdh’,
    inlined from ‘dtls_ecdh_pre_master_secret’ at crypto.c:455:3:
ecc/ecc.h:50:9: warning: ‘pub_x’ may be used uninitialized [-Wmaybe-uninitialized]
   50 |         ecc_ec_mult(px, py, secret, resultx, resulty);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from crypto.c:34:
crypto.c: In function ‘dtls_ecdh_pre_master_secret’:
ecc/ecc.h:47:6: note: by argument 1 of type ‘const uint32_t *’ {aka ‘const unsigned int *’} to ‘ecc_ec_mult’ declared here
   47 | void ecc_ec_mult(const uint32_t *px, const uint32_t *py, const uint32_t *secret, uint32_t *resultx, uint32_t *resulty);
      |      ^~~~~~~~~~~
crypto.c:442:12: note: ‘pub_x’ declared here
  442 |   uint32_t pub_x[8];
      |            ^~~~~
In file included from crypto.c:34:
In function ‘ecc_ecdh’,
    inlined from ‘dtls_ecdh_pre_master_secret’ at crypto.c:455:3:
ecc/ecc.h:50:9: warning: ‘pub_y’ may be used uninitialized [-Wmaybe-uninitialized]
   50 |         ecc_ec_mult(px, py, secret, resultx, resulty);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from crypto.c:34:
crypto.c: In function ‘dtls_ecdh_pre_master_secret’:
ecc/ecc.h:47:6: note: by argument 2 of type ‘const uint32_t *’ {aka ‘const unsigned int *’} to ‘ecc_ec_mult’ declared here
   47 | void ecc_ec_mult(const uint32_t *px, const uint32_t *py, const uint32_t *secret, uint32_t *resultx, uint32_t *resulty);
      |      ^~~~~~~~~~~
crypto.c:443:12: note: ‘pub_y’ declared here
  443 |   uint32_t pub_y[8];
      |            ^~~~~
In file included from crypto.c:34:
In function ‘ecc_ecdh’,
    inlined from ‘dtls_ecdh_pre_master_secret’ at crypto.c:455:3:
ecc/ecc.h:50:9: warning: ‘priv’ may be used uninitialized [-Wmaybe-uninitialized]
   50 |         ecc_ec_mult(px, py, secret, resultx, resulty);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from crypto.c:34:
crypto.c: In function ‘dtls_ecdh_pre_master_secret’:
ecc/ecc.h:47:6: note: by argument 3 of type ‘const uint32_t *’ {aka ‘const unsigned int *’} to ‘ecc_ec_mult’ declared here
   47 | void ecc_ec_mult(const uint32_t *px, const uint32_t *py, const uint32_t *secret, uint32_t *resultx, uint32_t *resulty);
      |      ^~~~~~~~~~~
crypto.c:441:12: note: ‘priv’ declared here
  441 |   uint32_t priv[8];
      |            ^~~~

This fixes the compile issue if the intent is for key_sizeto always be 8 (well actually 32 bytes).

$ git diff
diff --git a/crypto.c b/crypto.c
index be65a44..24c99b7 100644
--- a/crypto.c
+++ b/crypto.c
@@ -444,6 +444,7 @@ int dtls_ecdh_pre_master_secret(unsigned char *priv_key,
   uint32_t result_x[8];
   uint32_t result_y[8];

+  assert(key_size == 8);
   if (result_len < key_size) {
     return -1;
   }