pqc-thunderbird / libgcrypt

GNU General Public License v2.0
1 stars 0 forks source link

-fanalyzer warning for mlkem private_key_size_bytes #61

Closed falko-strenzke closed 8 months ago

falko-strenzke commented 8 months ago
cipher/mlkem.c|608 col 18| warning: use of uninitialized value ‘private_key_size_bytes’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
||   608 |   bit_strength = bitstrength_from_private_size_bytes (private_key_size_bytes);
||       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||   ‘mlkem_decrypt’: events 1-6
||     |
||     |  583 | mlkem_decrypt (gcry_sexp_t *r_plain, gcry_sexp_t s_data, gcry_sexp_t keyparms)
||     |      | ^~~~~~~~~~~~~
||     |      | |
||     |      | (1) entry to ‘mlkem_decrypt’
||     |......
||     |  588 |   size_t private_key_size_bytes;
||     |      |          ~~~~~~~~~~~~~~~~~~~~~~
||     |      |          |
||     |      |          (2) region created on stack here
||     |      |          (3) capacity: 8 bytes
||     |......
||     |  593 |   if (!shared_secret)
||     |      |      ~
||     |      |      |
||     |      |      (4) following ‘false’ branch (when ‘shared_secret’ is non-NULL)...
||     |......
||     |  601 |   ec = private_key_from_sexp (
||     |      |        ~~~~~~~~~~~~~~~~~~~~~~~
||     |      |        |
||     |      |        (5) ...to here
||     |      |        (6) calling ‘private_key_from_sexp’ from ‘mlkem_decrypt’
||     |  602 |       keyparms, &private_key, &private_key_size_bytes, NULL);
||     |      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||     |
||     +--> ‘private_key_from_sexp’: events 7-10
||            |
||            |  301 | private_key_from_sexp (const gcry_sexp_t keyparms,
||            |      | ^~~~~~~~~~~~~~~~~~~~~
||            |      | |
||            |      | (7) entry to ‘private_key_from_sexp’
||            |......
||            |  306 |   return extract_opaque_mpi_from_sexp (keyparms,
||            |      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||            |      |          |
||            |      |          (8) following ‘false’ branch (when ‘param’ is NULL)...
||            |      |          (9) ...to here
||            |      |          (10) calling ‘extract_opaque_mpi_from_sexp’ from ‘private_key_from_sexp’
||            |  307 |                                        "/z",
||            |      |                                        ~~~~~
||            |  308 |                                        sk_p,
||            |      |                                        ~~~~~
||            |  309 |                                        sk_len_p,
||            |      |                                        ~~~~~~~~~
||            |  310 |                                        param != NULL ? &param->secret_key_bytes
||            |      |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||            |  311 |                                                      : (u16 *)NULL,
||            |      |                                                      ~~~~~~~~~~~~~~
||            |  312 |                                        _gcry_malloc_secure);
||            |      |                                        ~~~~~~~~~~~~~~~~~~~~
||            |
||            +--> ‘extract_opaque_mpi_from_sexp’: events 11-12
||                   |
||                   |  233 | extract_opaque_mpi_from_sexp (const gcry_sexp_t keyparms,
||                   |      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
||                   |      | |
||                   |      | (11) entry to ‘extract_opaque_mpi_from_sexp’
||                   |......
||                   |  249 |   if (ec)
||                   |      |      ~
||                   |      |      |
||                   |      |      (12) following ‘false’ branch (when ‘ec == 0’)...
||                   |
||                 ‘extract_opaque_mpi_from_sexp’: event 13
||                   |
|../src/gcrypt-int.h|554| 32:
||                   |  554 | #define mpi_get_nbits(a)       _gcry_mpi_get_nbits ((a))
||                   |      |                                ^~~~~~~~~~~~~~~~~~~~~~~~~
||                   |      |                                |
||                   |      |                                (13) ...to here
cipher/mlkem.c|253 col 14| note: in expansion of macro ‘mpi_get_nbits’
||                   |  253 |   data_len = mpi_get_nbits (sk);
||                   |      |              ^~~~~~~~~~~~~
||                   |
||                 ‘extract_opaque_mpi_from_sexp’: events 14-21
||                   |
||                   |  254 |   if (data_len % 8)
||                   |      |      ^
||                   |      |      |
||                   |      |      (14) following ‘false’ branch...
||                   |......
||                   |  258 |   data_len /= 8;
||                   |      |   ~~~~~~~~~~~~~
||                   |      |            |
||                   |      |            (15) ...to here
||                   |  259 |   if (expected_size_bytes_opt
||                   |      |      ~
||                   |      |      |
||                   |      |      (16) following ‘false’ branch (when ‘expected_size_bytes_opt’ is NULL)...
||                   |......
||                   |  265 |   *data_p = alloc_func (data_len);
||                   |      |             ~~~~~~~~~~~~~~~~~~~~~
||                   |      |             |
||                   |      |             (17) ...to here
||                   |  266 |   if (*data_p == NULL)
||                   |      |      ~
||                   |      |      |
||                   |      |      (18) following ‘true’ branch...
||                   |  267 |     {
||                   |  268 |       ec = gpg_err_code_from_syserror ();
||                   |      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||                   |      |            |
||                   |      |            (19) ...to here
||                   |......
||                   |  288 |   if (ec)
||                   |      |      ~
||                   |      |      |
||                   |      |      (20) following ‘false’ branch (when ‘ec == 0’)...
||                   |......
||                   |  293 |   return ec;
||                   |      |          ~~
||                   |      |          |
||                   |      |          (21) ...to here
||                   |
||            <------+
||            |
||          ‘private_key_from_sexp’: event 22
||            |
||            |  306 |   return extract_opaque_mpi_from_sexp (keyparms,
||            |      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||            |      |          |
||            |      |          (22) returning to ‘private_key_from_sexp’ from ‘extract_opaque_mpi_from_sexp’
||            |  307 |                                        "/z",
||            |      |                                        ~~~~~
||            |  308 |                                        sk_p,
||            |      |                                        ~~~~~
||            |  309 |                                        sk_len_p,
||            |      |                                        ~~~~~~~~~
||            |  310 |                                        param != NULL ? &param->secret_key_bytes
||            |      |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||            |  311 |                                                      : (u16 *)NULL,
||            |      |                                                      ~~~~~~~~~~~~~~
||            |  312 |                                        _gcry_malloc_secure);
||            |      |                                        ~~~~~~~~~~~~~~~~~~~~
||            |
||     <------+
||     |
||   ‘mlkem_decrypt’: events 23-26
||     |
||     |  601 |   ec = private_key_from_sexp (
||     |      |        ^~~~~~~~~~~~~~~~~~~~~~~
||     |      |        |
||     |      |        (23) returning to ‘mlkem_decrypt’ from ‘private_key_from_sexp’
||     |  602 |       keyparms, &private_key, &private_key_size_bytes, NULL);
||     |      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||     |  603 |   if (ec)
||     |      |      ~  
||     |      |      |
||     |      |      (24) following ‘false’ branch (when ‘ec == 0’)...
||     |......
||     |  608 |   bit_strength = bitstrength_from_private_size_bytes (private_key_size_bytes);
||     |      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
||     |      |                  |
||     |      |                  (25) ...to here
||     |      |                  (26) use of uninitialized value ‘private_key_size_bytes’ here
||     |
falko-strenzke commented 8 months ago

False positive with the same exact pattern as #60.